[PATCH v9 0/7] refs backend preamble

2015-07-21 Thread David Turner
This reroll addresses Michael Haggerty's comments:

- Error messages are now in the form error: reason
- We no longer unnecessarily set errno in write_ref_to_lockfile
- Corrected a spelling error in the new docs and another in the tests
- Corrected some copypasta in a test.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 0/7] refs backend preamble

2015-07-21 Thread Michael Haggerty
On 07/21/2015 08:44 AM, David Turner wrote:
 This reroll addresses Michael Haggerty's comments:
 
 - Error messages are now in the form error: reason
 - We no longer unnecessarily set errno in write_ref_to_lockfile
 - Corrected a spelling error in the new docs and another in the tests
 - Corrected some copypasta in a test.

Isn't it sobering that, every time one looks at source code, one finds
things that could be improved? (Not just yours but all source code!)
Happily it seems like the items are getting ever more trivial, and
either with or without the last round of suggestions I think the code is
correct and is a nice step forward.

Reviewed-by: Michael Haggerty mhag...@alum.mit.edu

Off topic: I wonder whether sooner or later we will need a

git reflog remove ref

to delete an existing reference's reflog entirely without deleting the
reference and without assuming a filesystem backend. (Note that this is
different than `git reflog expire`, which leaves the empty reflog file
around and thus doesn't undo a --create-reflog.) But perhaps there is no
call for that operation. In a pinch a user could delete the reference
and recreate it to get rid of its reflog.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 0/7] refs backend preamble

2015-07-21 Thread David Turner
On Tue, 2015-07-21 at 13:49 -0700, Junio C Hamano wrote:
 David Turner dtur...@twopensource.com writes:
 
  Junio, now that Michael has marked this as reviewed, do you want to pull
  from github, or do you want me to send a re-roll to the mailing list?
 
 Let's see the final round; that would give me a chance to properly
 sign-off your patch, and also give others a chance to eyeball them
 for one last time.
 
 Thanks.

Sent, thanks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 0/7] refs backend preamble

2015-07-21 Thread Junio C Hamano
David Turner dtur...@twopensource.com writes:

 Junio, now that Michael has marked this as reviewed, do you want to pull
 from github, or do you want me to send a re-roll to the mailing list?

Let's see the final round; that would give me a chance to properly
sign-off your patch, and also give others a chance to eyeball them
for one last time.

Thanks.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 0/7] refs backend preamble

2015-07-21 Thread David Turner
On Tue, 2015-07-21 at 13:20 -0700, Michael Haggerty wrote:
 On 07/21/2015 08:44 AM, David Turner wrote:
  This reroll addresses Michael Haggerty's comments:
  
  - Error messages are now in the form error: reason
  - We no longer unnecessarily set errno in write_ref_to_lockfile
  - Corrected a spelling error in the new docs and another in the tests
  - Corrected some copypasta in a test.
 
 Isn't it sobering that, every time one looks at source code, one finds
 things that could be improved? (Not just yours but all source code!)

Indeed.

 Happily it seems like the items are getting ever more trivial, and
 either with or without the last round of suggestions I think the code is
 correct and is a nice step forward.
 
 Reviewed-by: Michael Haggerty mhag...@alum.mit.edu
 
 Off topic: I wonder whether sooner or later we will need a
 
 git reflog remove ref
 
 to delete an existing reference's reflog entirely without deleting the
 reference and without assuming a filesystem backend. (Note that this is
 different than `git reflog expire`, which leaves the empty reflog file
 around and thus doesn't undo a --create-reflog.) But perhaps there is no
 call for that operation. In a pinch a user could delete the reference
 and recreate it to get rid of its reflog.

Some of the tests do delete reflogs (manually), and the refs backend
code has a test driver that performs that operation (along with some
others).  I could move git reflog remove out of that driver and into git
reflog, but I'm reluctant to do it just for the tests.  If there's a
non-test reason for it, I'll be happy to.

I've pushed a new version of this series to
https://github.com/dturner-tw/git/ on the pluggable-refs-preamble
branch.  The new version addresses Michael's comments from this around,
and makes a variable static as you suggested (and said you would squash
in).

Junio, now that Michael has marked this as reviewed, do you want to pull
from github, or do you want me to send a re-roll to the mailing list?

Thanks.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html