On Tue, Jul 13, 2010 at 10:11:05PM +0200, Stefan Sperling wrote:
> Review below.
A couple of additional remarks:
Playing with svnrdump and comparing its output to the output of
svnadmin dump --deltas, I noticed that:
- svnrdump doesn't dump revision 0.
It should dump revision 0, because that revision can contain important
revprops such as metadata for svnsync (svn:sync-last-merge-rev etc.)
- You're missing a couple of fields:
The UUID of the repository.
Text-content-sha1
Text-delta-base-md5
Text-delta-base-sha1
- I've seen a "Prop-delta: true" line which svnadmin dump does not print.
- You're missing some newlines that svnadmin dump prints (cosmetic,
but it would be nice if both produced matching output).
How to reproduce what I'm seeing:
Use svnsync to get a copy of the numptyphysics repository at
https://vcs.maemo.org/svn/numptyphysics (I had a dump of that lying
around... other repositories might do the job just as well, of course)
Dump the repository using svnadmin dump --deltas.
Dump the repository using svnrdump.
Compare output with diff -u.
Please get rid of all global variables in svnrdump.c:
subversion/svnrdump/svnrdump.c:43: warning: declaration of `pool' shadows a glob
al declaration
subversion/svnrdump/svnrdump.c:33: warning: shadowed declaration is here
subversion/svnrdump/svnrdump.c:91: warning: declaration of `pool' shadows a glob
al declaration
subversion/svnrdump/svnrdump.c:33: warning: shadowed declaration is here
When adding unit tests for svnrdump, please make each and every one of
those tests compare with output of svnadmin dump --deltas, so that we
will keep them in sync.
Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html