Re: [PATCH 0/5] Fix verify_lock() to report errors via strbuf

From: Stefan Beller <sbeller@google.com>
Date: May 23, 2015, 00:15 UTC
Message-ID: <CAGZ79kYa7hcb1xejp+aS3cELh4RBbm9+Um_r_4UvEu9C1yyPhw@mail.gmail.com>
In-Reply-To: <1432337697-29161-1-git-send-email-mhagger@alum.mit.edu>

On Fri, May 22, 2015 at 4:34 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 21 quoted lines

> verify_lock() is a helper function called while committing reference
> transactions. But when it fails, instead of recording its error
> message in a strbuf to be passed back to the caller of
> ref_transaction_commit(), the error message was being written directly
> to stderr.
>
> Instead, report the errors via a strbuf so that they make it back to
> the caller of ref_transaction_commit().
>
> The last two patches remove the capitalization of some error messages,
> to be consistent with our usual practice. These are a slight backwards
> incompatibility; if any scripts are trying to grep for these error
> message strings, they might have to be adjusted. So feel free to drop
> them if you think consistency across time is more important than
> consistency across commands.
>
> This is the patch series that I mentioned here [1]. It applies on top
> of mh/ref-directory-file [2] and is thematically a continuation of
> that series in the sense that it further cleans up the error handling
> within reference transactions. It would be easy to rebase to master if
> that is preferred.

I was confused at first as I only looked at the patches and the corresponding line numbers did not match with the files as currently open in my editor. (they are roughly origin/master)

Now that I read the cover letter I can explain the line number being slightly different. :)

The series looks good to me.

Show 22 quoted lines

>
> These patches are also available on my GitHub account [3] as branch
> "verify-lock-strbuf-err".
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/269006
> [2] http://thread.gmane.org/gmane.comp.version-control.git/268778
> [3] https://github.com/mhagger/git
>
> Michael Haggerty (5):
>   verify_lock(): return 0/-1 rather than struct ref_lock *
>   verify_lock(): on errors, let the caller unlock the lock
>   verify_lock(): report errors via a strbuf
>   verify_lock(): do not capitalize error messages
>   ref_transaction_commit(): do not capitalize error messages
>
>  refs.c                | 40 ++++++++++++++++++++++++++--------------
>  t/t1400-update-ref.sh | 14 +++++++-------
>  2 files changed, 33 insertions(+), 21 deletions(-)
>
> --
> 2.1.4
>