On Thu, 2011-02-17, Noorul Islam K M wrote:
> Noorul Islam K M <noorul_at_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.
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.
- 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);
Received on 2011-02-17 16:41:35 CET