Re: [PATCH] Implement svnadmin verify --force
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 1 Nov 2012 18:50:13 +0000 (GMT)
Prabhu, some more review comments on your last patch.
Instead of checking for errors in several places within the main loop, put the verification code in a wrapper function and check for errors exactly once where you call that function. That way you won't miss any. In your last patch, there is at least one call that could return a verification error that isn't checked -- the call to svn_fs_revision_root().
Run the test suite before posting a patch, and mention the results. Three of the svnadmin tests fail when I run them with your patch:
FAIL: svnadmin_tests.py 11: verify a repository containing paths like 'c:hi'
Look for compiler warnings, and address any that indicate problems. I see two:
subversion/libsvn_repos/dump.c:1363:1: no previous prototype for 'notify_verification_error'
Include the log message each time you send the patch, even if the log message is the same as it was the previous time, so we don't have to hunt for it.
This is an archived mail posted to the Subversion Dev mailing list.