Bert, Karl,
thank you for your feedback.
On Saturday 06 September 2008 Bert Huijben wrote:
> This line and two other lines in this patch are more than 79 characters
> long.
Fixed.
> > + * converted by the functions \ref svn_time_to_cstring() and
> > + * \ref svn_time_from_cstring(). */
> > +#define SVN_PROP_TEXT_TIME SVN_PROP_PREFIX "text-time"
> > +
> > +/** The files' owner.
> > + * Stored as numeric ID, followed by whitespace, optionally followed by
> > the + * string: \c "1000 pmarek". Parsers \b should accept any number of
> > whitespace, + * and writers \b should put exactly a single space. */
> > +#define SVN_PROP_OWNER SVN_PROP_PREFIX "owner"
>
> Why is the unix specific numeric ID required and the more generic username
> optional?
If you're restoring 5 files with different owners, but your /etc/passwd
doesn't list them (think desaster recovers - you're trying to get
/etc/passwd back out of your repository :-), you should at least
get the 5 different UIDs back.
(Same behaviour with tar and rsync.)
> And on the comment: Why is the whitespace required if you can leave the
> username out? :)
You're right - I changed the text; the whitespace is optional too.
On Saturday 06 September 2008 Karl Fogel wrote:
> Philipp Marek <philipp_at_marek.priv.at> writes:
> > (Has someone manually put merge information for the branches in the
> > repository, to allow automatic use of "svn merge"?)
>
> I think so (er, not sure, ask in #svn-dev maybe?).
I'm a bit afraid of just using the svn_props.h file from trunk, because
that could create unnecessary conflicts on a merge later; but OTOH, the
meta-data branches are too old for automatic merging anyway. There'd be
a lot to do manually - more or less a rewrite, I think.
> For the issue, just give the URL or "issue #1256", no need to do both.
Fixed.
> Is there any other header in SVN trunk that uses this "\c" syntax,
No, you're right; subversion uses the "@" specifier for doxygen.
$ grep -r @c --include "*.h" . | wc -l
1256
$ grep -r @c --include "*.h" .
...
.../svn_config.h:169: * categories (@c SVN_CONFIG_CATEGORY_SERVERS,
> or the "\b"
I only found this here: svn_sorts.h
* If @a and @b intersect then the range with the lower start revision
* is considered the lesser range. If the ranges' start revisions are
* equal then the range with the lower end revision is considered the
* lesser range.
*
* @since New in 1.5
*/
int svn_sort_compare_ranges(const void *a, const void *b);
But that's wrong - the function above use @c or @a, and @param would be
possible too.
For my patch I'm using @c.
> and "\ref" syntaxes that you use below? I assume they're
> typesetting commands, but I think we should not introduce them if they
> are not already being used in trunk.
These are the doxygen highlightning and link-generations specifications
(http://www.stack.nl/~dimitri/doxygen/commands.html); "ref" isn't yet used
in subversion, so I substituted that with "@c".
I changed the \e that I used, too.
> This part about how they're not used on trunk should be at the top of
> the comment, and maybe with "***" around it or something to make it more
> prominent.
Fixed; I used the "===" lines from "copyright" above.
> > +#define SVN_PROP_TEXT_TIME SVN_PROP_PREFIX "text-time"
> Would it be better to call this "SVN_PROP_TEXT_LAST_MODTIME", or
> something involving the word "MOD", to make it clear exactly what it
> means? (Same with the value.)
I thought about that for some time. But
1) that constants is used in several files in several projects,
2) the documentation above clearly states that it's the "modification"
time,
3) there are quite so filesystems which have only a single filetime,
4) and versioning an "access" or "inode change" time (which are changed by
reading resp. eg. chmod) wouldn't make much sense, I think.
So I took the lazy route, and just kept that as-is.
If it's important for you, I can still change it. (It's just a fair amount
of work afterwards.)
> It's fine for you to commit it to trunk, after incorporating any
> feedback (e.g., Bert's comments, my comments below).
Thank you; I tried, but got "Authentication failed".
Either user "pmarek" is revoked for the subversion repository completely,
or at least limited to the meta-data branch.
I'm attaching the new patch.
Regards,
Phil
--
Versioning your /etc, /home or even your whole installation?
Try fsvs (fsvs.tigris.org)!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-07 13:27:07 CEST