On Fri, 2 Dec 2005 kfogel@collab.net wrote:
> lundblad@tigris.org writes:
> In rNNNNN, I made a few minor tweaks. Some comments/questions:
>
Thanks for doing that!
> > --- trunk/subversion/libsvn_wc/README (original)
> > +++ trunk/subversion/libsvn_wc/README Thu Dec 1 15:53:52 2005
> > @@ -123,114 +130,45 @@
> >
> > [...]
> >
> > +`dir-prop-revert':
> > + This is the base-props for this directory for the deleted directory
> > + in a schedule replace situation. If this file doesn't exist, the
> > + `dir-prop-base' file is used.
> > +
> > `lock'
>
> This wasn't completely clear to me: "...for this directory for the
> deleted directory in a schedule replace situation". Would this be
> equally correct?:
>
> In a schedule-replace situation for this directory, this holds the
> base-props for the deleted version of the directory (i.e., the
> version that is being replaced). If this file doesn't exist, the
> `dir-prop-base' file is used.
>
> If you approve, I can make that change.
>
That seems correct to me. Someone who worked with wc-replacements maybe
could double-check.
> > @@ -366,10 +308,199 @@
> > just internal tracking data used by the system, like the 'entries'
> > file.
> >
> > +The entries file
> > +----------------
> > +
> > +The entries file contains an XML document with a top-level element
> > +named `wc-entries'. This element contains one or more `entries'
> > +elements, one for each directory entry. The first `entry' element is
> > +the this dir entry with values for the directory containing this admin
> > +area.
>
> Two things:
>
> There is no requirement that the "this dir" entry be first. Rather,
> its identifying characteristic is that it's entry name is the empty
> string.
>
Well, you're right. *We* put it first in our code. I think that's good
since one could stop parsing the entries file after having read this_dir
if that is all that's wanted (I thinking of this like a status operation
of depth 0 which some GUI clients have shown interest in). But that's
just an optimization opportunity. Our code will not break if this_dir
isn't first.
> Second, in this explanation, we should probably set off the words
> "this dir" somehow, otherwise it reads oddly.
>
It is better after your tweak.
> > +`committed-rev':
> > + The last committed revision for this entry if this entry is in the
> > + repository. NO default; optional.
>
> Wait -- if this is optional, isn't the default the this_dir entry's
> committed-rev? Or the this_dir entry's base-rev? Or *something*? :-)
>
Nope. That would be strange; at least not so useful. Look at
take_from_entry in libsvn_wc/entries.c. That's where the "inheritance"
takes place.
> > +`committed-date':
> > + The date of the `committed-rev' if available. No default;
> > + optional.
> > +
> > +`last-author':
> > + The author of the `committed-rev' if available. No default;
> > + optional.
>
> Hmmm. But these are optional too... Maybe I'm wrong about
> committed-rev?
>
They are optional a) because committed-rev is and b) because date and
author may not be available for all revisions.
Thanks again for looking this over and fixing stuff for me,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 3 00:06:23 2005