From: David Turner <dturner@twopensource.com>
Date: Wed, 08 Jul 2015 20:01:56 GMT
Subject: Re: [PATCH v7 1/8] refs.c: add err arguments to reflog functions
Message-ID: <1436385716.4542.11.camel@twopensource.com>
In-Reply-To: <559D0B42.1040607@alum.mit.edu>

On Wed, 2015-07-08 at 13:36 +0200, Michael Haggerty wrote:
> On 07/08/2015 02:55 AM, David Turner wrote:
> > Add an err argument to log_ref_setup that can explain the reason
> > for a failure. This then eliminates the need to manage errno through
> > this function since we can just add strerror(errno) to the err string
> > when meaningful. No callers relied on errno from this function for
> > anything else than the error message.
> > 
> > Also add err arguments to private functions write_ref_to_lockfile,
> > log_ref_write_1, commit_ref_update. This again eliminates the need to
> > manage errno in these functions.
> > 
> > Some error messages change slightly.  For instance, we sometimes lose
> > "cannot update ref" and instead only show the specific cause of ref
> > update failure.
> 
> Did you check that the new error messages are at least as clear to the
> user as the old ones? (Sometimes errors from deep in the call stack
> provide details but don't make it clear how the details connect back to
> the action that the user was trying to do.)

They seem clear enough to me, but I guess there's no reason not to keep
the "cannot update ref" bit.  Will fix, along with the rest.