git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Eric Rannaud <e@nanocritical.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Jeremy Serror <jeremy.serror@gmail.com>
Subject: Re: [PATCH v4] fast-import: checkpoint: only write out refs and tags if we changed them
Date: Mon, 27 May 2019 16:27:13 -0700	[thread overview]
Message-ID: <CABPp-BFkqyGT_YHReKPJKy-nX5eWYhb9AVE2UeD7M+nhKT0dqg@mail.gmail.com> (raw)
In-Reply-To: <CAH_=xoZ5Lt2nn50cfDbTA-ZVeshi8HZ91Od4btuUXpBGCanGng@mail.gmail.com>

Hi Eric,

On Mon, May 27, 2019 at 3:46 PM Eric Rannaud <e@nanocritical.com> wrote:
> On Mon, May 27, 2019 at 2:12 PM Elijah Newren <newren@gmail.com> wrote:
> > On Fri, May 24, 2019 at 4:10 PM Eric Rannaud <e@nanocritical.com> wrote:
>
> I think the test will not fail without the patch (and therefore won't
> catch a regression): while checkpoint doesn't currently check the
> failure flag, it will be checked when cmd_main() returns so
> exit_status will be 1.

Ooh, good catch.

> You either want to:
> - (a) add more independent commands after the checkpoint and check
> that they were not run,
> - or (b) run with --done, do not include a done command, and check
> that fast-import does exit (but it's racy),
> - or (c) you can reuse background_import to have a more explicit
> sequence of events (in which case improvements to background_import
> from my patch would have to be committed first).

That sounds good...though it's taking my short patch and just about
amounts to completely rewriting it.  Would you like to take it over
including authorship, and just add either a "Original-patch-by:" or
"Based-on-patch-by:" for me in the commit message (these two tags
appear to be the two most common attribution mechanism used in
git.git's history when someone does this)?

> > +       cat >input <<-INPUT_END &&
> > +       feature done
> > +       commit refs/heads/V3
> > +       mark :3
> > +       committer Me My <self@and.eye> 1234567890 +0123
>
> You likely want to use:
> committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE

I see other tests in that testsuite using this, and using it here
certainly wouldn't hurt; I'm not opposed to it.  But I'm
curious...other than "other tests in the same testcase use it a lot" I
don't see why the choice of committer name/email/date matters at all.
Is there an actual reason for this that I just missed?

Cheers,
Elijah

  reply	other threads:[~2019-05-27 23:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24 23:09 [PATCH v4] fast-import: checkpoint: only write out refs and tags if we changed them Eric Rannaud
2019-05-25  2:18 ` Eric Rannaud
2019-05-27 21:12   ` Elijah Newren
2019-05-27 22:45     ` Eric Rannaud
2019-05-27 23:27       ` Elijah Newren [this message]
2019-05-28  0:11         ` Eric Rannaud

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=CABPp-BFkqyGT_YHReKPJKy-nX5eWYhb9AVE2UeD7M+nhKT0dqg@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=e@nanocritical.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeremy.serror@gmail.com \
    /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).