git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Devin J. Pohly" <djpohly@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Charles Bailey <cbailey32@bloomberg.net>,
	git@vger.kernel.org
Subject: Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits
Date: Fri, 3 Mar 2017 02:55:35 -0500	[thread overview]
Message-ID: <20170303075535.rxij4knd7d4ayfev@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqshmvbhe5.fsf@gitster.mtv.corp.google.com>

On Thu, Mar 02, 2017 at 11:36:18AM -0800, Junio C Hamano wrote:

> "Devin J. Pohly" <djpohly@gmail.com> writes:
> 
> > I think your point is interesting too, though.  If a commit is also
> > TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems
> > reasonable that someone might want to leave it in the filtered branch as
> > an empty commit while pruning empt*ied* commits.  I would imagine that
> > as another option (--prune-newly-empty?).
> 
> I was hoping to hear from others who may care about filter-branch to
> comment on this topic to help me decide, but I haven't heard
> anything, so here is my tentative thinking.
> 
> I am leaning to:
> 
>  * Take your series as-is, which would mean --prune-empty will
>    change the behaviour to unconditionally lose the empty root.
> 
>  * Then, people who care deeply about it can add a new option that
>    prunes commits that become empty while keeping the originally
>    empty ones.
> 
> Thoughts?

Sorry, this was on my to-review list but the sha1 stuff has been much
more exciting. :)

The behavior that Devin proposes is what I would have expected to
happen. Between "prune-existing-empty" and "prune-became-empty", I've
never had a use for the distinction. But as I think this is similar to
the way cherry-pick distinguished between "redundant" and "empty", I
guess some people have.

I agree that it should be a new, separate option, as it's orthogonal to
dealing with the root commit (i.e., somebody is equally likely to want
to preserve an already-empty commit from the middle of history).

The change to filter-branch itself looks obviously correct. The only
objectionable thing I noticed in the test additions is that the early
ones should be marked test_expect_failure until the fix from 3/4 flips
them to "success". Otherwise it breaks bisectability.

-Peff

  parent reply	other threads:[~2017-03-03  8:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23  8:27 [PATCH 1/4] t7003: ensure --prune-empty can prune root commit Devin J. Pohly
2017-02-23  8:27 ` [PATCH 2/4] t7003: ensure --prune-empty removes entire branch when applicable Devin J. Pohly
2017-02-23  8:27 ` [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits Devin J. Pohly
2017-02-23 21:17   ` Junio C Hamano
2017-02-23 21:33     ` Devin J. Pohly
2017-03-02 19:36       ` Junio C Hamano
2017-03-02 21:18         ` Devin J. Pohly
2017-03-02 21:39           ` Junio C Hamano
2017-03-02 23:28         ` Jacob Keller
2017-03-03  7:55         ` Jeff King [this message]
2017-03-03 20:30           ` Devin J. Pohly
2017-03-03 20:43           ` Junio C Hamano
2017-02-23  8:27 ` [PATCH 4/4] p7000: add test for filter-branch with --prune-empty Devin J. Pohly
2017-03-03  7:56   ` Jeff King
2017-03-03 20:34     ` Devin J. Pohly

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=20170303075535.rxij4knd7d4ayfev@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=cbailey32@bloomberg.net \
    --cc=djpohly@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /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).