git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: David Turner <dturner@twopensource.com>
To: Michael Haggerty <mhagger@alum.mit.edu>,
	Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v9 0/7] refs backend preamble
Date: Tue, 21 Jul 2015 16:43:24 -0400	[thread overview]
Message-ID: <1437511404.30911.21.camel@twopensource.com> (raw)
In-Reply-To: <55AEA998.60501@alum.mit.edu>

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 <mhagger@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.

  reply	other threads:[~2015-07-21 20:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-21 15:44 [PATCH v9 0/7] refs backend preamble David Turner
2015-07-21 15:44 ` [PATCH v9 1/7] refs.c: add err arguments to reflog functions David Turner
2015-07-21 19:50   ` Michael Haggerty
2015-07-21 15:44 ` [PATCH v9 2/7] refs: Break out check for reflog autocreation David Turner
2015-07-21 15:45 ` [PATCH v9 3/7] refs: new public ref function: safe_create_reflog David Turner
2015-07-21 19:57   ` Michael Haggerty
2015-07-21 15:45 ` [PATCH v9 4/7] git-reflog: add exists command David Turner
2015-07-21 19:51   ` Michael Haggerty
2015-07-21 15:45 ` [PATCH v9 5/7] refs: add REF_FORCE_CREATE_REFLOG flag David Turner
2015-07-21 15:45 ` [PATCH v9 6/7] update-ref and tag: add --create-reflog arg David Turner
2015-07-21 16:42   ` Junio C Hamano
2015-07-21 20:02     ` Michael Haggerty
2015-07-21 20:06       ` Michael Haggerty
2015-07-21 15:45 ` [PATCH v9 7/7] git-stash: use update-ref --create-reflog instead of creating files David Turner
2015-07-21 20:20 ` [PATCH v9 0/7] refs backend preamble Michael Haggerty
2015-07-21 20:43   ` David Turner [this message]
2015-07-21 20:49     ` Junio C Hamano
2015-07-21 21:05       ` David Turner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1437511404.30911.21.camel@twopensource.com \
    --to=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).