[PATCH]: APU DBD documentation cleanup (some)

[Bojan Smojver <bo...@rexursive.com>]

Before I commit this, I would like to make sure that additional  
explanations are useful and based on real behaviour of DBD. There is  
some simplification in relation to apr_dbd_prepare() syntax, but  
that's on purpose.

-- 
Bojan

Re: [PATCH]: APU DBD documentation cleanup (some)

[Garrett Rooney <ro...@electricjellyfish.net>]

On 5/5/06, Bojan Smojver <bo...@rexursive.com> wrote:
> On Fri, 2006-05-05 at 13:47 +0100, Nick Kew wrote:
>
> > On apr_dbd_open, your remarks about specific drivers don't really
> > belong there, IMHO.  The connection string documentation is already
> > in the manual:
> > http://httpd.apache.org/docs/2.2/mod/mod_dbd.html#dbdparams
>
> Hmm, not sure why APR/APU users would necessarily read Apache manual.
> For instance, Subversion folks may decide they like DBD, in which case
> they need to know how to use the drivers in question. Personally, I use
> APR DBD directly as well, not through mod_dbd. And, if they use Doxygen
> to document their APIs, there could be links to APU docs, in which case
> it would be handy to have this there.

I have to agree with Bojan here, the fact that it's documented in the
mod_dbd docs isn't enough, there's no reason a user of APR should have
to look there.

-garrett

Re: [PATCH]: APU DBD documentation cleanup (some)

[Bojan Smojver <bo...@rexursive.com>]

On Fri, 2006-05-05 at 13:47 +0100, Nick Kew wrote:

> On apr_dbd_open, your remarks about specific drivers don't really
> belong there, IMHO.  The connection string documentation is already
> in the manual:
> http://httpd.apache.org/docs/2.2/mod/mod_dbd.html#dbdparams

Hmm, not sure why APR/APU users would necessarily read Apache manual.
For instance, Subversion folks may decide they like DBD, in which case
they need to know how to use the drivers in question. Personally, I use
APR DBD directly as well, not through mod_dbd. And, if they use Doxygen
to document their APIs, there could be links to APU docs, in which case
it would be handy to have this there.

> transaction_start: Your remark is correct, but is just documenting
> standard APR behaviour.  There's a more important programming
> consideration: for a module to rely on this cleanup is wrong,
> because it will pollute the connection for any other module that
> uses the connection whilst processing the same request.

OK, get it - it's just a fallback. I'll drop that.

> Your second remark is also correct, but that's likely to change
> as we're updating transaction behaviour.

Yes, I know. I was planning to update that once I test the new
transaction behaviour (hey, my patch actually compiles now :-).

However, folks that assume that transactions are going to behave like
the underlying database they are using (e.g. SQLite, Oracle or MySQL)
may get surprised when the whole transaction fails instead of partially
succeeding. So, I wanted to let them know about this.

> prepare: That's OK, but very specific, and will call for updating
> whenever a driver is updated.

I would think that this is a good thing, actually. If there is specific
behaviour that a driver supports, users of the API should be made aware
how to trigger it. Therefore, we should document it for them.

> Instead of the final sentence,
> how about something more generic like "Some drivers may
> support different data types using printf-like format: for example
> %d or %f for numeric data."

If you find that my comment above is missing the mark, I'll change this.

Thanks for reviewing the patch!

-- 
Bojan

Re: [PATCH]: APU DBD documentation cleanup (some)

[Nick Kew <ni...@webthing.com>]

On Friday 05 May 2006 03:24, Bojan Smojver wrote:
> Before I commit this, I would like to make sure that additional
> explanations are useful and based on real behaviour of DBD. There is
> some simplification in relation to apr_dbd_prepare() syntax, but
> that's on purpose.

Hmmm, some comments.  Don't take this as a thumbs down
(except perhaps the one about the transaction cleanup);
just food for thought.

On apr_dbd_open, your remarks about specific drivers don't really
belong there, IMHO.  The connection string documentation is already
in the manual:
http://httpd.apache.org/docs/2.2/mod/mod_dbd.html#dbdparams

transaction_start: Your remark is correct, but is just documenting
standard APR behaviour.  There's a more important programming
consideration: for a module to rely on this cleanup is wrong,
because it will pollute the connection for any other module that
uses the connection whilst processing the same request.
Your second remark is also correct, but that's likely to change
as we're updating transaction behaviour.

prepare: That's OK, but very specific, and will call for updating
whenever a driver is updated.  Instead of the final sentence,
how about something more generic like "Some drivers may
support different data types using printf-like format: for example
%d or %f for numeric data."


-- 
Nick Kew