Re: [PATCH RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Hi Junio, Junio C Hamano writes: Fabian Ruch baf...@gmail.com writes: It makes the next patch easier to understand because the finalising command line git commit --allow-empty --amend --no-post-rewrite -n --no-edit seems to be simply moved to the end of do_pick. Substituting --no-edit for -C there would make that log message overly long ... And the reason why the same -C $1 is not used and --no-edit is used in the final version that is moved to the end of do_pick is...? The finalising command line is executed by do_pick to rewrite $1 when it is being replayed. For instance, one purpose of rewriting is amending the current head with the changes introduced by $1. In this case, we don't want the log message of $1 to be the final log message. Another purpose of rewriting is using a text file or a string as log message, which would be defeated by -C $1 as well. It's true that, by the time the next patch is introduced, do_pick doesn't yet implement either rewriting mechanism (they are both used to thread squash through do_pick later on) but these considerations suggest that -C $1 might not be what we want to use to disable the log message editor. Fabian -- 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 RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Hi Junio,
Junio C Hamano writes:
Fabian Ruch baf...@gmail.com writes:
The command line used to recreate root commits specifies the
effectless option `-C`. It is used to reuse commit message and
authorship from the named commit but the commit being amended here,
which is the sentinel commit, already carries the authorship and log
message of the processed commit. Note that the committer email and
commit date fields do not match the root commit either way. Remove
the option. However, `-C` (other than `-c`) does not invoke the
editor and the `--amend` option invokes it by default. Disable editor
invocation again by specifying `--no-edit`.
I'd say this is a no-value change, in the sense that it can be
written either way for the same effect and if the original were
written with --amend --no-edit then there would be no reason to
change it to -C $1 because such a change would also be equally a
no-value change.
What exactly are we gaining? Performance? Correctness?
I submitted this change separately from the next (rebase -i: Commit
only once when rewriting picks) for the following reasons, at least I
thought they were.
It makes the next patch easier to understand because the finalising
command line git commit --allow-empty --amend --no-post-rewrite -n
--no-edit seems to be simply moved to the end of do_pick. Substituting
--no-edit for -C there would make that log message overly long and the
paragraph saying why this substitution is correct would be distracting
(it's a little unfortunate that there is this special case in the first
place). While the resulting history stays the same of course, it might
be confusing to someone reading the code that the log message gets
replaced with itself.
I know the last point is rather weak but aren't the two patches and log
messages easier to read?
Fabian
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ff04d5d..17836d5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
git commit --allow-empty \
- --amend --no-post-rewrite -n -C $1 \
+ --amend --no-post-rewrite -n --no-edit \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
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 RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Fabian Ruch baf...@gmail.com writes: It makes the next patch easier to understand because the finalising command line git commit --allow-empty --amend --no-post-rewrite -n --no-edit seems to be simply moved to the end of do_pick. Substituting --no-edit for -C there would make that log message overly long ... And the reason why the same -C $1 is not used and --no-edit is used in the final version that is moved to the end of do_pick is...? -- 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 RFC v2 08/19] rebase -i: Root commits are replayed with an unnecessary option
Fabian Ruch baf...@gmail.com writes:
The command line used to recreate root commits specifies the
effectless option `-C`. It is used to reuse commit message and
authorship from the named commit but the commit being amended here,
which is the sentinel commit, already carries the authorship and log
message of the processed commit. Note that the committer email and
commit date fields do not match the root commit either way. Remove
the option. However, `-C` (other than `-c`) does not invoke the
editor and the `--amend` option invokes it by default. Disable editor
invocation again by specifying `--no-edit`.
I'd say this is a no-value change, in the sense that it can be
written either way for the same effect and if the original were
written with --amend --no-edit then there would be no reason to
change it to -C $1 because such a change would also be equally a
no-value change.
What exactly are we gaining? Performance? Correctness?
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ff04d5d..17836d5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
git commit --allow-empty \
---amend --no-post-rewrite -n -C $1 \
+--amend --no-post-rewrite -n --no-edit \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
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
