[PATCH] Work Queue doc strings and comments

[Julian Foad <ju...@btopenworld.com>]

Are these doc strings and comments good?

- Julian

Re: [PATCH] Work Queue doc strings and comments

[Julian Foad <ju...@wandisco.com>]

On Fri, 2010-03-19 at 10:49 -0400, Greg Stein wrote:
> On Fri, Mar 19, 2010 at 08:36, Julian Foad <ju...@wandisco.com> wrote:
> > I (Julian Foad) wrote:
> >> Are these doc strings and comments good?
> >
> > Sometimes I don't use enough words.
> >
> > I believe these additions are helpful, not so much because they provide
> > high density and hard-to-discover info (they don't, yet), but because
> > they provide a solid starting point for expanding as the functions
> > evolve to become more permanent and more complex.  Having these as
> > starting points lowers the bar for adding documentation of any
> > non-trivial behaviours by indicating that some consideration has already
> > been made of the basics, and provides a quicker reference for the reader
> > than the alternative which was to search up and down the file for where
> > and how the function was used as a clue to its purpose and API.
> >
> > So all I'm asking is: is there anything incorrect or a very bad idea in
> > this patch?
> 
> Nothing wrong with it,

Great. r925253.

>  but I believe ALL of the current work items
> will disappear. As I've mentioned before, I think they are "backwards"
> from the desired design.

Yup, I've made a note of that and will try to assist with changing them
around.  I'm absolutely fine with them all being deleted, comments and
all.

> They are handy in terms of how loggy used to operate, but they do not
> make sense in our new database world.
> 
> These work items perform DB work *and* file work. They should only
> perform file manipulations. Further, the work items should be
> installed as part of a DB transaction, within wc_db. For example, when
> code calls svn_wc__db_op_delete(), the database changes will be
> performed to delete the node(s) and work item(s) will be
> transactionally inserted to (then) make the filesystem reflect the
> state within the database (e.g remove prop files, the working copy
> file/dir, etc).

Great.  That makes sense to me.

> So while there is nothing inherently bad/wrong with the comments, they
> don't help much.

They help me and others to understand the current state of play, not
much, I agree, but a bit.

> I'm about to start writing a "real" work item that will install a
> working copy file from its pristine text and its translation props.
> This is to correct a problem we have today where prop changes are
> being made and it tries to install the working copy file mid-state.
> The database is inconsistent, so the file-based work is seeing bad
> data. I need a work item to do the installation *after* the database
> state is stable. This same work item will be used to install files
> during checkout/update/switch and during post-commit, so it will be
> "real" and will be used for quite a bit of things.

Cool.

> Once I finish that work item, if it is not documented to your desires,
> *then* I'd highly recommend jumping in and clarifying anything about
> it that bothers you.

:-)

Thanks.
- Julian

Re: [PATCH] Work Queue doc strings and comments

[Greg Stein <gs...@gmail.com>]

On Fri, Mar 19, 2010 at 08:36, Julian Foad <ju...@wandisco.com> wrote:
> I (Julian Foad) wrote:
>> Are these doc strings and comments good?
>
> Sometimes I don't use enough words.
>
> I believe these additions are helpful, not so much because they provide
> high density and hard-to-discover info (they don't, yet), but because
> they provide a solid starting point for expanding as the functions
> evolve to become more permanent and more complex.  Having these as
> starting points lowers the bar for adding documentation of any
> non-trivial behaviours by indicating that some consideration has already
> been made of the basics, and provides a quicker reference for the reader
> than the alternative which was to search up and down the file for where
> and how the function was used as a clue to its purpose and API.
>
> So all I'm asking is: is there anything incorrect or a very bad idea in
> this patch?

Nothing wrong with it, but I believe ALL of the current work items
will disappear. As I've mentioned before, I think they are "backwards"
from the desired design.

They are handy in terms of how loggy used to operate, but they do not
make sense in our new database world.

These work items perform DB work *and* file work. They should only
perform file manipulations. Further, the work items should be
installed as part of a DB transaction, within wc_db. For example, when
code calls svn_wc__db_op_delete(), the database changes will be
performed to delete the node(s) and work item(s) will be
transactionally inserted to (then) make the filesystem reflect the
state within the database (e.g remove prop files, the working copy
file/dir, etc).

So while there is nothing inherently bad/wrong with the comments, they
don't help much.

I'm about to start writing a "real" work item that will install a
working copy file from its pristine text and its translation props.
This is to correct a problem we have today where prop changes are
being made and it tries to install the working copy file mid-state.
The database is inconsistent, so the file-based work is seeing bad
data. I need a work item to do the installation *after* the database
state is stable. This same work item will be used to install files
during checkout/update/switch and during post-commit, so it will be
"real" and will be used for quite a bit of things.

Once I finish that work item, if it is not documented to your desires,
*then* I'd highly recommend jumping in and clarifying anything about
it that bothers you.

Cheers,
-g

Re: [PATCH] Work Queue doc strings and comments

[Julian Foad <ju...@wandisco.com>]

I (Julian Foad) wrote:
> Are these doc strings and comments good?

Sometimes I don't use enough words.

I believe these additions are helpful, not so much because they provide
high density and hard-to-discover info (they don't, yet), but because
they provide a solid starting point for expanding as the functions
evolve to become more permanent and more complex.  Having these as
starting points lowers the bar for adding documentation of any
non-trivial behaviours by indicating that some consideration has already
been made of the basics, and provides a quicker reference for the reader
than the alternative which was to search up and down the file for where
and how the function was used as a clue to its purpose and API.

So all I'm asking is: is there anything incorrect or a very bad idea in
this patch?

- Julian