Hi Karl,
Since you mentioned that you were planning to merge the
sparse-directories branch to trunk shortly, I thought I'd take a quick
look at the branch diff. I've not reviewed it thoroughly, but here's
some random comments:
Meta-comments:
- There's an awful lot of TODOs, but I get the sense that they're intended
to be exhaustive rather than indicate where you think there's a problem
that absolutely needs fixing.
- Is the intention to get sparse directories to work completely for 1.5,
or just to get it to work at least as well as -N currently?
- If the latter, the only blocker for release seems to be the work needed
to make a client with a sparse wc ignore data sent by sparse-ignorant
servers. What exactly is the status of this? Is this something that's
absolutely necessary for 1.5?
ra_svn:
- ra_svn: In the 'update', 'switch', 'status', and 'diff' commands,
we're adding the depth inside a tuple, whereas in the 'set-path' and
'link-path' cases we're not. The former matches what the extensibility
parts of the protocol document suggest, but the latter makes more sense
to me (I don't understand what the protocol document is saying here).
Regardless, can we pick one approach?
- libsvn_ra_svn/protocol is missing the changes to all commands apart from
set-path and link-path.
- svnserve/serve.c:diff() - You've added an optional depth tuple inside
code that checks for a 1.4 request (one without text_deltas).
You should set depth_word to NULL directly rather than rely on
svn_ra_svn_parse_tuple to do it for you - it's just confusing as-is.
Other:
- (This isn't exclusive to sparse-directories: changelist and keep-local
do it too, but:) Why are we writing out the depth attribute in the
old XML-format entries, when by definition it can never have any value
other than the default? (Also note that the specification for the
entries XML format prohibits unknown attributes, so this is
technically wrong).
(Actually, could someone remind me why we even have the capability to
write out the XML entries format? I'm sure there is a reason, but I
can't remember what it is.)
- The help for the command-line option --depth doesn't mention that
'infinity' is a valid option (though it is referenced in the help for
--recursive).
- If we're updating the XML output for 'svn info', we should update the
schema too (in subversion/schema/info.rnc).
- svn_types.h: Contrary to the comments, you use svn_depth_unknown all
over the place. However, you don't use svn_depth_exclude -- are you
fairly sure you'll need it, or would it be better to get rid of it
until a need comes up?
- libsvn_wc/entries.h has a reference to non-existent svn_depth_ignore.
- Looking at the branch diff for include/svn_ra.h, I notice you deleted
a lot of comments of the form "the RA layer assumes that all paths are
relative to the URL used to open @a session." (see r21314). Any reason?
It seems unrelated to the other changes.
Regards,
Malcolm
- application/pgp-signature attachment: stored
Received on Thu Mar 15 18:41:43 2007