Folks,
I seriously think that it is time for me to take a break from sending
iterative patches and just sit back and analyse the whole problem from
first. I value all your precious times spent on this and I wish to come
back with a much cleaner work which would abide by the coding standards
of the subversion community. Now that I have got more and more advices
and ideas from you guys, I guess I need to digest them all and come back
with a cleaner work for this feature.
Special thanks to Daniel, Stefan, Julian and Mike. Will get back soon
with a cleanly analysed work.
Thanks and regards
Prabhu
On 11/05/2012 07:38 PM, Julian Foad wrote:
Prabhu Gnana Sundar<prabh...@collab.net> wrote:
Julian Foad wrote:
[...] We should choose either "notification" style
(that is, messages that are not error messages), such as
[...]
or "error messages" style, in which case the messages should be
formatted like all svn err msgs, for example:
[...]
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.
Oh... Why did you choose that and not C-Mike Pilato's follow-up suggestion?
But, I am also adding one more error message as follows
svnadmin: E165005: Repository has corruptions.
and then exit with exit code 1.
Great, that sounds good, apart from what somebody suggested about changing the
wording to something like 'failed to verify'.
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.
I think when --keep-going is false we should still emit this new error -- in
addition to whatever it already emitted. That would change the existing
behaviour a bit, but I think in a good way, because it would be more consistent.
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.
The log message helps people to review the patch, by explaining the reason for
the changes. Sorry, but I am not willing to review your patch without a log
message.
Also, it would
really help me (and other reviewers) if you would write responses to the review
comments. There have been lots of comments, probably many more than you were
expecting. I can't tell whether you have read them all and addressed them all
in your latest patch, or if you have not yet had enough time to work through
them.
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 ?
Yes, it is OK to tweak the expectations of existing tests when we change some
details of the existing behaviour.
- Julian
