[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r19299 - branches/nonxml-entries/subversion/libsvn_wc

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2006-04-10 23:52:37 CEST

Hi Peter,

Random review comments, hope you don't mind.

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?).

> 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?

> +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?

> +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.

> +
> +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'?

> +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?

> +XML-based entries format

> +
> +Fields added in format 7 and later are not allowed in the XML-based
> +format.

(are there any?)

> 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.

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 10 23:53:11 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.