Malcolm Rowe writes:
> Random review comments, hope you don't mind.
I always mind when people look at what I do, but in a very positive
sense. Thanks for having a look.
> On Mon, Apr 10, 2006 at 12:38:10PM -0700, lundblad@tigris.org wrote:
> > ON the nonxml-entries branch:
> >
> > --- branches/nonxml-entries/subversion/libsvn_wc/README (original)
> > +++ branches/nonxml-entries/subversion/libsvn_wc/README Mon Apr 10 12:38:08 2006
> > @@ -111,13 +111,9 @@
> >
> > Says what version of the working copy adm format this is (so future
> > clients can be backwards compatible easily).
> > - The changes in each format are listed in wc.h.
> >
> > - Also, the presence of this file means that the entire process of
> > - creating the adm area was completed, because this is always the
> > - last file created. Of course, that's no guarantee that someone
> > - didn't muck things up afterwards, but it's good enough for
> > - existence-checking.
> > + This file is deprecated and present for backwards compatibility.
> > + It may be removed in the future.
> >
>
> Why delete that text? It's all still true, and (IMO) useful (or are there
> better ways to portably test whether the admin area is ready to use?).
I meant to move this to the section on the entries file, because we're
hopefully phasing out the format file in some later release. So the
last significant file created from now on is the entries file.
> > The entries file
> > ----------------
> >
>
> > +
> > +The characters "|", "\" and ASCII control characters (0x01 - 0x31 and
> > +0x7f) must be escaped when they occur in a field. The escaping uses
>
> Why 0x7f?
Because it is an ASCII control character (DEL). There's no point in
making an exception for it. It should be *veeeeeery* uncommon.
> > +the syntax "\xHH", where "HH" are the two hexadecimal digits
> > +representing the escaped byte. The character encoding of the entries
>
> Lowercase or uppercase hex-encoding, or is either allowed?
Both. I clarified this.
> > +representing the escaped byte. The character encoding of the entries
> > +file is UTF-8, with no leading BOM allowed.
>
> That's not overly clear; perhaps it should be at the start of the section,
> so we don't (i.e. _I_ don't) get confused about whether we're talking
> about file encoding or field encoding.
See the new paragraph about the general file format (line separation
and character encoding).
>
> > +
> > +A field may be boolean, in which case it can have one of the values
> > +'t`, meaning true or 'f`, meaning false. Unless otherwise specified,
> > +boolean fields default to `f' if emtpy. Timestamps are stored in
> > +the format produced by svn_time_to_cstring().
> >
>
> We don't have any booleans that default to true, do we? Ditch 'Unless
> otherwise specified'?
We may want that in the future (maybe) and I want all boolean fields
to be represented the same.
> > +checksum:
> > For file entries, base-64-encoded MD5 checksum of the text-base
> > file. Optional, for backwards compatibility.
> >
>
> This is a new file format, so there is no backwards compatibility to
> worry about: can't we just start mandating the checksum from now?
Would be nice, but it would require some work. The checksum isn't
always present when writing the entry to disk. I'm +1 if anyone puts
in the work to do it :-).
> > +XML-based entries format
>
> > +
> > +Fields added in format 7 and later are not allowed in the XML-based
> > +format.
>
> (are there any?)
But there may be in the future and I'm sure that someone who just adds
a field to the list in the previous section will forget about this old
cruft, so better prevent this confusion now...
> > The attributes `has-props', `has-prop-mods', `cachable-props',
> > +and `present-props' are only valid in format 6.
>
> That's rather confusing, since they're also valid in format 7.
>
This format is not used in format 7, so they can't be valid. Note the
difference between field and attribute in this description. But if
you have a way to describe it clearer, feel free to suggest or just
change, of course:-)
r19304 incorporates some of your suggestions. Thanks again, Malcolm.
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 11 08:26:14 2006