[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Implement svnadmin verify --force

From: Prabhu Gnana Sundar <prabhugs_at_collab.net>
Date: Sat, 03 Nov 2012 22:49:21 +0530

>>>>> But the code moves the E160004 away from its current location
>>>>> immediately after the "svnadmin:" prefix. I am not sure I like that;
>>>>> is there a good reason not to have the message be of the form
>>>>> svnadmin: E160004: %s
>>>>> in the interest of parseability?
>>>> I agree that would be better: the prefix should remain just
>>>> "svnadmin: " and the error message should be adjusted instead.
>>> Does this look fine ? I personally feel this is easily readable.
>>>
>>> * Verified revision 0.
>>> * Verified revision 1.
>>> * Verified revision 2.
>>> Error verifying revision 3. svnadmin: E160004: Missing node-id in node-rev
>> at r3 (offset 787)
>>> Error verifying revision 4. svnadmin: E140001: zlib (uncompress): corrupt
>> data: Decompression of svndiff data failed
>>> * Verified revision 5.
> That's easily readable, but I don't like it: it's a funny mixture of styles. We should choose either "notification" style (that is, messages that are not error messages), such as
>
> * Verified revision 0.
> * Verified revision 1.
> * Verified revision 2.
> * Error verifying revision 3: Missing node-id in node-rev at r3 (offset 787)
> * Error verifying revision 4: zlib (uncompress): corrupt data: Decompression of svndiff data failed
> * Verified revision 5.
>
> or "error messages" style, in which case the messages should be formatted like all svn err msgs, for example:
>
> * Verified revision 0.
> * Verified revision 1.
> * Verified revision 2.
> svnadmin: E199999: Error verifying revision 3
> svnadmin: E160004: Missing node-id in node-rev at r3 (offset 787)
> svnadmin: E199999: Error verifying revision 4
> svnadmin: E140001: zlib (uncompress): corrupt data: Decompression of svndiff data failed
> * Verified revision 5.
>
> But the output-style design here is linked to the question of what we're going to print at the end of the verification. Prabhu, what's the plan for when the command exits? Print a summary or an error message? Exit with non-zero return code (I hope)?

I made the output to be exactly like the one you have suggested above.
But, I am also adding one more error message as follows

svnadmin: E165005: Repository has corruptions.

and then exit with exit code 1. The default behaviour is not at all
affected. This new error message is effected only when --keep-going is
true. Is that fine ? Now all the test cases pass.

Attaching the patch for you to have a look at it.

Not attaching the log message since I am yet to work on sending the
"Verified revision x" to the stdout. I am facing a few issues in it.
Like, a few tests fail because the expected output don't match with the
actual output. So need suggestions on that too. Is it fine to tweak the
already available test case's expected output based on the output of
what we send to stdout and stderr ?

Thanks and regards
Prabhu

Received on 2012-11-03 18:27:55 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.