Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: jonathantanmy@google.com, Johannes.Schindelin@gmx.de,
	bturner@atlassian.com, sami@boukortt.com,
	Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH v3 0/4] rebase -i: mark commits that begin empty in todo editor
Date: Tue, 14 Apr 2020 10:11:45 +0100
Message-ID: <2ba9f811-3589-3284-cd4b-0ea9c315a2c0@gmail.com> (raw)
In-Reply-To: <pull.757.v3.git.git.1586573067.gitgitgadget@gmail.com>

Hi Elijah

On 11/04/2020 03:44, Elijah Newren via GitGitGadget wrote:
> Changes since v2:
> 
>   * Addressed feedback from Junio in patch 2, including moving some of the
>     wording to the new patch (see next point)
>   * Edited jt/rebase-allow-duplicate to include the changes Jonathan
>     mentioned on the list, then rebased it on top of this series, and made
>     the related options mention each other.
> 
> Elijah Newren (3):
>    rebase -i: mark commits that begin empty in todo editor

I think this is a useful addition

>    rebase: reinstate --no-keep-empty

This looks like a good compromise - we're still changing the default but 
keeping the ability to drop commits that start empty. One thought on 
this patch - previously we would comment out the empty commits in the 
todo list so the user could decide what to do. Now we just don't add 
them to the list so the user has no way to selectively add them back. I 
guess this doesn't matter too much as they can use the new '# empty' 
label and comment out the commits they want to drop rather than 
uncommenting the ones they want to keep

>    rebase: fix an incompatible-options error message

good catch!

Best Wishes

Phillip

> 
> Jonathan Tan (1):
>    rebase --merge: optionally skip upstreamed commits
> 
>   Documentation/git-rebase.txt      | 70 +++++++++++++++++++++-------
>   builtin/rebase.c                  | 24 +++++++---
>   sequencer.c                       | 14 +++++-
>   sequencer.h                       |  4 +-
>   t/t3402-rebase-merge.sh           | 77 +++++++++++++++++++++++++++++++
>   t/t3421-rebase-topology-linear.sh | 10 ++--
>   t/t3424-rebase-empty.sh           | 36 +++++++++++++++
>   7 files changed, 202 insertions(+), 33 deletions(-)
> 
> 
> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-757%2Fnewren%2Frebase-mark-empty-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-757/newren/rebase-mark-empty-v3
> Pull-Request: https://github.com/git/git/pull/757
> 
> Range-diff vs v2:
> 
>   1:  0d94eea376a = 1:  0d94eea376a rebase -i: mark commits that begin empty in todo editor
>   2:  e15c599c874 ! 2:  3a5bedc68d6 rebase: reinstate --no-keep-empty
>       @@ Commit message
>        
>            Commit d48e5e21da ("rebase (interactive-backend): make --keep-empty the
>            default", 2020-02-15) turned --keep-empty (for keeping commits which
>       -    start empty) into the default.  That commit viewed this, though as
>       -    turning that flag into a no-op.  As such, --no-keep-empty was translated
>       -    into undoing a no-op, i.e. also a no-op.  The logic underpinning that
>       -    commit was:
>       +    start empty) into the default.  The logic underpinning that commit was:
>        
>              1) 'git commit' errors out on the creation of empty commits without an
>                 override flag
>       @@ Commit message
>            look really weird despite making perfect sense (the first will drop
>            commits which become empty, but keep commits that started empty; the
>            second will keep commits which become empty, but drop commits which
>       -    started empty).
>       +    started empty).  However, --no-keep-empty was named years ago and we are
>       +    predominantly keeping it for backward compatibility; also we suspect it
>       +    will only be used rarely since folks already have a simple way to drop
>       +    commits they don't want with an interactive rebase.
>        
>            Reported-by: Bryan Turner <bturner@atlassian.com>
>            Reported-by: Sami Boukortt <sami@boukortt.com>
>       @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
>        -always dropped.
>        +Note that commits which start empty are kept (unless --no-keep-empty
>        +is specified), and commits which are clean cherry-picks (as determined
>       -+by `git log --cherry-mark ...`) are detected and dropped as a preliminary
>       -+step (unless --keep-cherry-picks is passed).
>       ++by `git log --cherry-mark ...`) are always dropped.
>         +
>         See also INCOMPATIBLE OPTIONS below.
>         
>       @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
>        -	to halt.
>        +	Do not keep commits that start empty before the rebase
>        +	(i.e. that do not change anything from its parent) in the
>       -+	result.  For commits which become empty after rebasing, see
>       -+	the --empty and --keep-cherry-pick flags.
>       ++	result.  The default is to keep commits which start empty,
>       ++	since creating such commits requires passing the --allow-empty
>       ++	override flag to `git commit`, signifying that a user is very
>       ++	intentionally creating such a commit and thus wants to keep
>       ++	it.
>         +
>        -See also BEHAVIORAL DIFFERENCES and INCOMPATIBLE OPTIONS below.
>       ++Usage of this flag will probably be rare, since you can get rid of
>       ++commits that start empty by just firing up an interactive rebase and
>       ++removing the lines corresponding to the commits you don't want.  This
>       ++flag exists as a convenient shortcut, such as for cases where external
>       ++tools generate many empty commits and you want them all removed.
>       +++
>       ++For commits which do not start empty but become empty after rebasing,
>       ++see the --empty flag.
>       +++
>        +See also INCOMPATIBLE OPTIONS below.
>         
>         --allow-empty-message::
>   3:  ee5e42361fc = 3:  5c8863b9d34 rebase: fix an incompatible-options error message
>   -:  ----------- > 4:  20d3a50f5a4 rebase --merge: optionally skip upstreamed commits
> 

      parent reply index

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-09 23:26 [PATCH] " Elijah Newren via GitGitGadget
2020-04-10  0:50 ` Junio C Hamano
2020-04-10  2:06   ` Bryan Turner
2020-04-10  4:57     ` Junio C Hamano
2020-04-10 17:51 ` [PATCH v2 0/3] " Elijah Newren via GitGitGadget
2020-04-10 17:51   ` [PATCH v2 1/3] " Elijah Newren via GitGitGadget
2020-04-10 17:51   ` [PATCH v2 2/3] rebase: reinstate --no-keep-empty Elijah Newren via GitGitGadget
2020-04-10 20:37     ` Junio C Hamano
2020-04-10 21:41       ` Elijah Newren
2020-04-10 17:51   ` [PATCH v2 3/3] rebase: fix an incompatible-options error message Elijah Newren via GitGitGadget
2020-04-10 20:42   ` [PATCH v2 0/3] rebase -i: mark commits that begin empty in todo editor Junio C Hamano
2020-04-10 21:04     ` Junio C Hamano
2020-04-10 21:14       ` Junio C Hamano
2020-04-10 22:11         ` Elijah Newren
2020-04-10 21:29     ` Junio C Hamano
2020-04-10 22:13       ` Elijah Newren
2020-04-10 22:30         ` Junio C Hamano
2020-04-11  0:07           ` Elijah Newren
2020-04-11 21:14             ` Junio C Hamano
2020-04-11  2:44   ` [PATCH v3 0/4] " Elijah Newren via GitGitGadget
2020-04-11  2:44     ` [PATCH v3 1/4] " Elijah Newren via GitGitGadget
2020-04-15 20:52       ` Junio C Hamano
2020-04-11  2:44     ` [PATCH v3 2/4] rebase: reinstate --no-keep-empty Elijah Newren via GitGitGadget
2020-04-11  2:44     ` [PATCH v3 3/4] rebase: fix an incompatible-options error message Elijah Newren via GitGitGadget
2020-04-11  2:44     ` [PATCH v3 4/4] rebase --merge: optionally skip upstreamed commits Jonathan Tan via GitGitGadget
2020-04-11 18:12       ` Jonathan Tan
2020-04-14  9:11     ` Phillip Wood [this message]

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

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

  git send-email \
    --in-reply-to=2ba9f811-3589-3284-cd4b-0ea9c315a2c0@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=bturner@atlassian.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sami@boukortt.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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git