Peter N. Lundblad wrote:
> I've been trying to give libsvn_wc/README some massaging. [...]
Thanks. I've finally got around to looking at this, weeks after you checked it
in. It's certainly much needed and valuable. I read through and made a few
comments in line.
> Index: subversion/libsvn_wc/README
> --- subversion/libsvn_wc/README (revision 17563)
> +++ subversion/libsvn_wc/README (arbetskopia)
> @@ -79,38 +79,45 @@
> .svn/format /* Contains wc adm format version. */
> entries /* Various adm info for each directory entry */
> - dir-props /* Working properties for this directory */
> + dir-props /* Optional, Working properties for this
> + directory */
Using the word "optional" makes it sound like the properties might be listed in
this file but they might not (with no clue as to why). Perhaps the intended
meaning, that the file is present if and only if there are some properties,
would be better expressed by "if any" at the end of the sentence, and/or a
brief reference to a footnote that describes this aspect. (See below about the
> - empty-file /* An empty file, for convenience. */
> + empty-file /* An empty file, for convenience, not used. */
This is confusing: how is the file convenient if it is not used? Perhaps say
"(until format 4)" instead if that is correct.
> + This files was added in format 4 and earlier. This was used to
"This files" -> "This 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'
At the end of that line: "`entries'" -> "`entry'".
> +elements, one for each directory entry. The first `entry' element is
> +the this dir entry with values for the directory containing this admin
Put quotes around "this dir" to make it read properly, here and everywhere
below. Or a good alternative would be to hyphenate it, which would be less clumsy.
> +The `entry' element is empty and has the attributes described below.
> +an attribute may be boolean, in which case it can have one of the
Start with a capital letter.
> +values `true' or `false'. Boolean attributes default to `false' if
> +not present. Tiestamps are stored in the format produced by
"Tiestamps" -> "Timestamps"
> + If this entry is added with history, the URL of the copy source.
> + May be present iff `copyfrom-rev' is present.
It would be clearer to remove "May be" from this sentence, if that is correct,
and similarly the one below.
> + If this entry is added with history, the revision of the copy
> + source. May be present iff `copyfrom-url' is present.
> +`conflict-old', `conflict-new' and `conflict-wrk':
> + In case of a text conflict, these three attributes specify the relative
> + filenames of the three saved conflict files. No defaults;
> + optional.
I think we need to be a bit careful about using the word "optional". I would
guess that in this case these three fields are always present when there is a
state of conflict and always absent otherwise. These fields occupy optional
attributes in the XML syntax, but in terms of their meaning to Subversion their
presence or absence is required and not optional.
Some other fields are similarly required to be present or absent according to
circumstance, whereas some others are indeed optional in that it doesn't matter
whether they exist or not. If you could review all uses of the word "optional"
in this list that would be great.
> +The only attributes allowed in an entry for a directory (other than
> +the this dir entry) are `name', `absent', `deleted', `schedule', `copied',
> +`copyfrom-url', `copyfrom-rev'. [...]
And "kind", presumably, as it is required on every entry.
To unsubscribe, e-mail: firstname.lastname@example.org
For additional commands, e-mail: email@example.com
Received on Wed Jan 11 02:32:45 2006