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<prabhugs_at_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
Received on 2012-11-05 16:37:44 CET