Julian Foad <julian.f...@wandisco.com> writes:
> On Thu, 2011-02-17, Noorul Islam K M wrote:
>
>> Noorul Islam K M <noo...@collab.net> writes:
>> Log
>> [[[
>>
>> Fix failing expected error regex. Also capture ra_neon error.
>
> Hi Noorul. Writing a good log message is quite a difficult skill to
> learn, so please don't be dismayed at my comment here.
>
Not at all. I am here to learn.
> You need to describe the change at a high level. In the context of the
> whole project, what is changing, and why? The line above just suggests
> that this fixes something to do with errors, but doesn't give me a clue
> whether it was a bug in Subversion or a bug in the tests, or what part
> of Subversion's behaviour is affected. Imagine that I am (or Kamesh or
> Stefan or Hyrum is) reading through the output of "svn log" to find
> commits related to bugs in merging, for example: the message should give
> me a clue about whether this change is likely to be relevant.
>
>> * subversion/svn/blame-cmd.c
>> (svn_cl__blame): Catch SVN_ERR_FS_NOT_FOUND and display warning.
>
> As a consequence of not being able to read what is the purpose of this
> patch, I have no idea why you are including this change.
>
Thank you very much. I will keep these points in mind and with help from
all you I will improve.
Thanks and Regards
Noorul
> - Julian
>
>
>> * subversion/tests/cmdline/blame_tests.py
>> (blame_non_existent_url_target): Relax regex to allow errors from
>> http: and svn: protocols.
>>
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> ]]]
>>
>> plain text document attachment (blame_relax_regex.txt)
>> Index: subversion/tests/cmdline/blame_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/blame_tests.py (revision 1071618)
>> +++ subversion/tests/cmdline/blame_tests.py (working copy)
>> @@ -759,8 +759,7 @@
>> " 2 jrandom New contents for iota\n",
>> ]
>>
>> - expected_err = "svn: warning: W160017: '/non-existent' " + \
>> - "is not a file in revision 2\n" + \
>> + expected_err = "svn: warning: (W160017|W160013): .*\n" + \
>> ".*\nsvn: E200009: Could not perform blame on all targets " + \
>> "because some targets don't exist\n"
>> expected_err_re = re.compile(expected_err)
>> Index: subversion/svn/blame-cmd.c
>> ===================================================================
>> --- subversion/svn/blame-cmd.c (revision 1071618)
>> +++ subversion/svn/blame-cmd.c (working copy)
>> @@ -379,7 +379,8 @@
>> target));
>> }
>> else if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND ||
>> - err->apr_err == SVN_ERR_FS_NOT_FILE)
>> + err->apr_err == SVN_ERR_FS_NOT_FILE ||
>> + err->apr_err == SVN_ERR_FS_NOT_FOUND)
>> {
>> svn_handle_warning2(stderr, err, "svn: ");
>> svn_error_clear(err);
