threads / patch / 39494

patch, 2 partsgit-rebase -i: add command "drop" to remove a commit

Subject: [PATCH/RFCv3 1/2] git-rebase -i: add command "drop" to remove a commit

## tl;dr

10 messages between Jun 2, 2015 and Jun 3, 2015. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Galan Rémi· Jun 2, 2015, 13:36 UTC · lore

Instead of removing a line to remove the commit, you can use the command "drop" (just like "pick" or "edit"). It has the same effect as deleting the line (removing the commit) except that you keep a visual trace of your actions, allowing a better control and reducing the possibility of removing a commit by mistake.

Signed-off-by: Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr>
---
 Documentation/git-rebase.txt  |  3 +++
 git-rebase--interactive.sh    | 18 ++++++++++++++++++
 t/lib-rebase.sh               |  4 ++--
 t/t3404-rebase-interactive.sh | 10 ++++++++++
 4 files changed, 33 insertions(+), 2 deletions(-)
Show changes to 4 files +33 −2

Documentation/git-rebase.txt, git-rebase--interactive.sh, t/lib-rebase.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d01baa..9cf3760 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -514,6 +514,9 @@ rebasing.
 If you just want to edit the commit message for a commit, replace the
 command "pick" with the command "reword".
 
+To drop a commit, replace the command "pick" with "drop", or just
+delete its line.
+
 If you want to fold two or more commits into one, replace the command
 "pick" for the second and subsequent commits with "squash" or "fixup".
 If the commits had different authors, the folded commit will be
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..869cc60 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -152,6 +152,7 @@ Commands:
  s, squash = use commit, but meld into previous commit
  f, fixup = like "squash", but discard this commit's log message
  x, exec = run command (the rest of the line) using shell
+ d, drop = remove commit
 
 These lines can be re-ordered; they are executed from top to bottom.
 
@@ -508,6 +509,23 @@ do_next () {
 	"$comment_char"*|''|noop)
 		mark_action_done
 		;;
+	drop|d)
+		if test -z $sha1
+		then
+			warn "Missing SHA-1 in 'drop' command."
+			die "Please fix this using 'git rebase --edit-todo'."
+		fi
+
+		sha1_verif="$(git rev-parse --verify --quiet $sha1^{commit})"
+		if test -z $sha1_verif
+		then
+			warn "'$sha1' is not a SHA-1 or does not represent" \
+				"a commit in 'drop' command."
+			die "Please fix this using 'git rebase --edit-todo'."
+		fi
+
+		mark_action_done
+		;;
 	pick|p)
 		comment_for_reflog pick
 
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6bd2522..fdbc900 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -14,7 +14,7 @@
 #       specified line.
 #
 #   "<cmd> <lineno>" -- add a line with the specified command
-#       ("squash", "fixup", "edit", or "reword") and the SHA1 taken
+#       ("squash", "fixup", "edit", "reword" or "drop") and the SHA1 taken
 #       from the specified line.
 #
 #   "exec_cmd_with_args" -- add an "exec cmd with args" line.
@@ -46,7 +46,7 @@ set_fake_editor () {
 	action=pick
 	for line in $FAKE_LINES; do
 		case $line in
-		squash|fixup|edit|reword)
+		squash|fixup|edit|reword|drop)
 			action="$line";;
 		exec*)
 			echo "$line" | sed 's/_/ /g' >> "$1";;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ac429a0..8960083 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1102,4 +1102,14 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '
 	test $(git cat-file commit HEAD | sed -ne \$p) = I
 '
 
+test_expect_success 'drop' '
+	test_when_finished "git checkout master" &&
+	git checkout -b dropBranchTest master &&
+	set_fake_editor &&
+	FAKE_LINES="1 drop 2 3 drop 4 5" git rebase -i --root &&
+	test E = $(git cat-file commit HEAD | sed -ne \$p) &&
+	test C = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+	test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
+'
+
 test_done
-- 
2.4.1.411.g9c4ad60
Galan Rémi· Jun 2, 2015, 13:36 UTC · re: Galan Rémi · lore

[PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Check if commits were removed (i.e. a line was deleted) and print warnings or abort git rebase according to the value of the configuration variable rebase.missingCommitsCheckLevel.

Add the configuration variable rebase.missingCommitsCheckLevel.
    - When unset or set to "ignore", no checking is done.
    - When set to "warn", the commits are checked, warnings are
      displayed but git rebase still proceeds.
    - When set to "error", the commits are checked, warnings are
      displayed and the rebase is aborted.
rebase.missingCommitsCheckLevel defaults to "ignore".
Signed-off-by: Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr>
---
 Documentation/config.txt      |   7 +++
 Documentation/git-rebase.txt  |   6 +++
 git-rebase--interactive.sh    | 105 ++++++++++++++++++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |  25 ++++++++++
 4 files changed, 143 insertions(+)
Show changes to 4 files +143 −0

Documentation/config.txt, Documentation/git-rebase.txt, git-rebase--interactive.sh, t/t3404-rebase-interactive.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4d21ce1..ec9011d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2160,6 +2160,13 @@ rebase.autoStash::
 	successful rebase might result in non-trivial conflicts.
 	Defaults to false.
 
+rebase.missingCommitsCheckLevel::
+	If set to "warn", git rebase -i will print a warning if some
+	commits are removed (i.e. a line was deleted), however the
+	rebase will still proceed. If set to "error", it will print
+	the previous warning and abort the rebase. If set to
+	"ignore", no checking is done.  Defaults to "ignore".
+
 receive.advertiseAtomic::
 	By default, git-receive-pack will advertise the atomic push
 	capability to its clients. If you don't want to this capability
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9cf3760..71029f8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -213,6 +213,12 @@ rebase.autoSquash::
 rebase.autoStash::
 	If set to true enable '--autostash' option by default.
 
+rebase.missingCommitsCheckLevel::
+	If set to "warn" print warnings about removed commits in
+	interactive mode. If set to "error" print the warnings and
+	abort the rebase. If set to "ignore" no checking is
+	done. "ignore" by default.
+
 OPTIONS
 -------
 --onto <newbase>::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 869cc60..6391423 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -851,6 +851,109 @@ add_exec_commands () {
 	mv "$1.new" "$1"
 }
 
+# Print the list of the SHA-1 of the commits
+# from a todo list in a file.
+# $1: todo-file, $2: outfile
+todo_list_to_sha_list () {
+	todo_list=$(git stripspace --strip-comments <"$1")
+	temp_file=$(mktemp)
+	echo "$todo_list" >$temp_file
+	while read -r command sha1 rest
+	do
+		case $command in
+		x|"exec")
+			;;
+		*)
+			printf "%s\n" "$sha1"
+			;;
+		esac
+	done <$temp_file >"$2"
+	rm $temp_file
+}
+
+# Transforms SHA-1 list in argument
+# to a list of commits (in place)
+# Doesn't check if the SHA-1 are commits.
+# $1: file with long SHA-1 list
+long_sha_to_commit_list () {
+	short_missing=""
+	git_command="git show --oneline"
+	get_line_command="head -n 1"
+	temp_file=$(mktemp)
+	while read -r sha
+	do
+		if test -n "$sha"
+		then
+			commit=$($git_command $sha | $get_line_command)
+			if test -n "$commit"
+			then
+				printf "%s\n" "$commit"
+			fi
+		fi
+	done <"$1" >$temp_file
+	mv $temp_file "$1"
+}
+
+# Use warn for each line of a file
+# $1: file to warn
+warn_file () {
+	while read -r line
+	do
+		warn " - $line"
+	done <"$1"
+}
+
+# Check if the user dropped some commits by mistake
+# Behaviour determined by .gitconfig.
+check_commits () {
+	checkLevel=$(git config --get rebase.missingCommitsCheckLevel)
+	checkLevel=${checkLevel:-ignore}
+	# To lowercase
+	checkLevel=$(echo "$checkLevel" | tr 'A-Z' 'a-z')
+
+	case "$checkLevel" in
+	warn|error)
+		# Get the SHA-1 of the commits
+		todo_list_to_sha_list "$todo".backup "$todo".oldsha1
+		todo_list_to_sha_list "$todo" "$todo".newsha1
+
+		# Sort the SHA-1 and compare them
+		echo "$(sort -u "$todo".oldsha1)" >"$todo".oldsha1
+		echo "$(sort -u "$todo".newsha1)" >"$todo".newsha1
+		echo "$(comm -2 -3 "$todo".oldsha1 "$todo".newsha1)" >"$todo".miss
+
+		# Make the list user-friendly
+		long_sha_to_commit_list "$todo".miss
+
+		# Check missing commits
+		if test -s "$todo".miss
+		then
+			warn "Warning : some commits may have been dropped" \
+				"accidentally."
+			warn "Dropped commits (in no relevent order):"
+			warn_file "$todo".miss
+			warn ""
+			warn "To avoid this message, use \"drop\" to" \
+				"explicitly remove a commit."
+			warn "Use git --config rebase.missingCommitsCheckLevel to change" \
+				"the level of warnings (ignore,warn,error)."
+			warn ""
+
+			if test "$checkLevel" = error
+			then
+				die_abort "Rebase aborted due to dropped commits."
+			fi
+		fi
+		;;
+	ignore)
+		;;
+	*)
+		warn "Unrecognized setting $checkLevel for option" \
+			"rebase.missingCommitsCheckLevel in git rebase -i."
+		;;
+	esac
+}
+
 # The whole contents of this file is run by dot-sourcing it from
 # inside a shell function.  It used to be that "return"s we see
 # below were not inside any function, and expected to return
@@ -1096,6 +1199,8 @@ has_action "$todo" ||
 
 expand_todo_ids
 
+check_commits
+
 test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
 
 GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8960083..07a7158 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1112,4 +1112,29 @@ test_expect_success 'drop' '
 	test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
 '
 
+test_expect_success 'rebase -i respects rebase.missingCommitsCheckLevel=error' '
+	test_config rebase.missingCommitsCheckLevel error &&
+	test_when_finished "git checkout master &&
+		git branch -D tmp2" &&
+	git checkout -b tmp2 master &&
+	set_fake_editor &&
+	test_must_fail env FAKE_LINES="1 2 3 4" \
+		git rebase -i --root &&
+	test E = $(git cat-file commit HEAD | sed -ne \$p)
+'
+
+test_expect_success 'rebase -i respects rebase.missingCommitsCheckLevel=warn' '
+	test_config rebase.missingCommitsCheckLevel warn &&
+	test_when_finished "git checkout master &&
+		git branch -D tmp2" &&
+	git checkout -b tmp2 master &&
+	set_fake_editor &&
+	FAKE_LINES="1 2 3 4" \
+		git rebase -i --root 2>warn.tmp &&
+	test D = $(git cat-file commit HEAD | sed -ne \$p) &&
+	sed -n "1p" warn.tmp >warnl1.tmp &&
+	echo "Warning : some commits may have been dropped accidentally." >l1.tmp &&
+	test_cmp warnl1.tmp l1.tmp
+'
+
 test_done
-- 
2.4.1.411.g9c4ad60
Matthieu Moy· Jun 2, 2015, 14:56 UTC · re: Galan Rémi · lore

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
> Check if commits were removed (i.e. a line was deleted) and print
> warnings or abort git rebase according to the value of the
s/according to/depending on/
(although both translate to the same "selon" in french ;-))
> configuration variable rebase.missingCommitsCheckLevel.

Now I'm wondering whether rebase.missingCommits would be sufficient. The full config would be

[rebase]
	missingCommits = warn
which reads like "in rebase, on missing commit, warn me".
Show 8 quoted lines
> Add the configuration variable rebase.missingCommitsCheckLevel.
>     - When unset or set to "ignore", no checking is done.
>     - When set to "warn", the commits are checked, warnings are
>       displayed but git rebase still proceeds.
>     - When set to "error", the commits are checked, warnings are
>       displayed and the rebase is aborted.
>
> rebase.missingCommitsCheckLevel defaults to "ignore".

This all describes well *what* the commit is doing (which is essentially rendundant with the documentation actually), but fails to really explain *why*, which is the most important question to adress in a commit message.

I'm convinced that this is a good idea (partly because I'm the one who suggested ^^), but not everybody is.

Show 6 quoted lines
> +rebase.missingCommitsCheckLevel::
> +	If set to "warn", git rebase -i will print a warning if some
> +	commits are removed (i.e. a line was deleted), however the
> +	rebase will still proceed. If set to "error", it will print
> +	the previous warning and abort the rebase. If set to
> +	"ignore", no checking is done.  Defaults to "ignore".
I think the relationship with 'drop' should be clarified here.

"To drop a commit without warning or error, use the `drop` command in the todo-list."

?
Show 6 quoted lines
> +# Print the list of the SHA-1 of the commits
> +# from a todo list in a file.
> +# $1: todo-file, $2: outfile
> +todo_list_to_sha_list () {
> +	todo_list=$(git stripspace --strip-comments <"$1")
> +	temp_file=$(mktemp)

c5770f7 (contrib/diffall: create tmp dirs without mktemp, 2012-03-14) says "mktemp is not available on all platforms." ...

> +	echo "$todo_list" >$temp_file

Don't use echo on user-supplied data. It's not portable (think what happens if $todo_list starts with a -).

printf '%s' "$todo_list"

You don't need all these intermediate variables/files. It looks like you want

git stripspace --strip-comments | while read -r command sha1 rest do ...

> +# Transforms SHA-1 list in argument
> +# to a list of commits (in place)
> +# Doesn't check if the SHA-1 are commits.
s/if/whether/
Show 11 quoted lines
> +# $1: file with long SHA-1 list
> +long_sha_to_commit_list () {
> +	short_missing=""
> +	git_command="git show --oneline"
> +	get_line_command="head -n 1"
> +	temp_file=$(mktemp)
> +	while read -r sha
> +	do
> +		if test -n "$sha"
> +		then
> +			commit=$($git_command $sha | $get_line_command)
Why not git show --oneline $sha without using this $git_command?
To me
  git show --oneline $sha | head -n 1
says all that has to be said, while
  $git_command $sha | $get_line_command
does not say anything (although it uses more characters).

But actually, computing the diff with `git show` and eliminating it with `head` seems backwards.

I guess you want something like:
  git rev-list --pretty=oneline -1 $sha
And the whole long_sha_to_commit_list is more or less equivalent to
  git rev-list --stdin --no-walk --format=oneline --abbrev-commit
(Yes, git was _meant_ to be scriptable, and it really is)
> +# Use warn for each line of a file
> +# $1: file to warn
I don't parse "file to warn". I'd rather warn the user than a file ;-).
> +# Check if the user dropped some commits by mistake
> +# Behaviour determined by .gitconfig.

not necessarily .gitconfig, there are other names. Say rebase.missingCommitsCheckLevel if you want to be technically accurate.

Show 5 quoted lines
> +check_commits () {
> +	checkLevel=$(git config --get rebase.missingCommitsCheckLevel)
> +	checkLevel=${checkLevel:-ignore}
> +	# To lowercase
> +	checkLevel=$(echo "$checkLevel" | tr 'A-Z' 'a-z')
Good comments insist on _why_ not _what_. I'd write:
	# Don't be case sensitive
	checkLevel=$(echo "$checkLevel" | tr 'A-Z' 'a-z')
Show 9 quoted lines
> +	case "$checkLevel" in
> +	warn|error)
> +		# Get the SHA-1 of the commits
> +		todo_list_to_sha_list "$todo".backup "$todo".oldsha1
> +		todo_list_to_sha_list "$todo" "$todo".newsha1
> +
> +		# Sort the SHA-1 and compare them
> +		echo "$(sort -u "$todo".oldsha1)" >"$todo".oldsha1
> +		echo "$(sort -u "$todo".newsha1)" >"$todo".newsha1
Useless uses of echo.
echo $(foo) -> foo
> +			warn "Warning : some commits may have been dropped" \
No space before : in english (hmm, didn't I already notice this one?)
> +			warn "Use git --config rebase.missingCommitsCheckLevel to change" \
> +				"the level of warnings (ignore,warn,error)."
Spaces after commas, or (ignore/warn/error).
> +		warn "Unrecognized setting $checkLevel for option" \
> +			"rebase.missingCommitsCheckLevel in git rebase -i."

I'd drop the "in git rebase -i" part. The user may have run "git rebase --interactive" and not know -i, and normally already knows which command is executed.

Show 10 quoted lines
> +test_expect_success 'rebase -i respects rebase.missingCommitsCheckLevel=error' '
> +	test_config rebase.missingCommitsCheckLevel error &&
> +	test_when_finished "git checkout master &&
> +		git branch -D tmp2" &&
> +	git checkout -b tmp2 master &&
> +	set_fake_editor &&
> +	test_must_fail env FAKE_LINES="1 2 3 4" \
> +		git rebase -i --root &&
> +	test E = $(git cat-file commit HEAD | sed -ne \$p)
> +'
You should test also that

git rebase --edit-todo # playing with $EDITOR to restore the original lines. git rebase --continue

actually continues. You did have problems with this in early implementations, so it's not straightforward, so it deserves a test.

You should check the output of git rebase -i too.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Remi Galan Alfonso· Jun 2, 2015, 16:32 UTC · re: Matthieu Moy · lore

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 8 quoted lines
> > Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
> > +                # Sort the SHA-1 and compare them
> > +                echo "$(sort -u "$todo".oldsha1)" >"$todo".oldsha1
> > +                echo "$(sort -u "$todo".newsha1)" >"$todo".newsha1
> 
> Useless uses of echo.
> 
> echo $(foo) -> foo

In this case it is not true, because of the infile and outfile being identical. However sort does have a -o (-output) that I missed that allows avoiding using echo or writing in another file; I'm correcting with this.

Show 7 quoted lines
> You should test also that
> 
> git rebase --edit-todo # playing with $EDITOR to restore the original lines.
> git rebase --continue
> 
> actually continues. You did have problems with this in early
> implementations, so it's not straightforward, so it deserves a test.

In this patch, the error is dealt with a die_abort and not die, thus there is no rebase --edit-todo or rebase --continue possible afterward.

> You should check the output of git rebase -i too.

Checking that the warning was correctly displayed like in the test for "warn" if I understood correctly. About that, is checking that the first line is "Warning: some commits may have been dropped accidentally." (like in the test for "warn") enough, or should I check that the commit displayed as removed is the correct one?

The other points have been corrected, or their correction is in progress.

Thanks,
Rémi
Junio C Hamano· Jun 2, 2015, 18:22 UTC · re: Remi Galan Alfonso · lore

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:

> In this case it is not true, because of the infile and outfile being
> identical. However sort does have a -o (-output) that I missed that
> allows avoiding using echo or writing in another file; I'm correcting
> with this.

Even though it is in POSIX, some implementation may lack it, so our code tend to avoid "sort -o". "echo $(sort)" is also ugly and inefficient. The obvious and old-fashioned would be sufficient here:

	sort -u "$todo".oldsha1 >"$todo".oldsha1+ &&
        mv "$todo".oldsha1+ "$todo".oldsha1
Matthieu Moy· Jun 3, 2015, 06:42 UTC · re: Remi Galan Alfonso · lore

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
Show 5 quoted lines
> Checking that the warning was correctly displayed like in the test for
> "warn" if I understood correctly. About that, is checking that the
> first line is "Warning: some commits may have been dropped
> accidentally." (like in the test for "warn") enough, or should I check
> that the commit displayed as removed is the correct one?

Ideally, you would check the list of commits displayed too. If the commits sha1 are stable, this should be easy to do. If it's too hard to test, I'd say its not worth the trouble, but others may disagree.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Remi Galan Alfonso· Jun 3, 2015, 07:33 UTC · re: Matthieu Moy · lore

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Ideally, you would check the list of commits displayed too. If the
> commits sha1 are stable, this should be easy to do. If it's too hard to
> test, I'd say its not worth the trouble, but others may disagree.

Originally I chose not to check if the SHA-1 were corrects since check_commits was called right after expand_todo_ids and I thought that expand_todo_ids checked them, but from what I understand, it doesn't seem to check if the SHA-1 are commits, I could be wrong though.

Rémi
Remi Galan Alfonso· Jun 3, 2015, 08:25 UTC · re: Remi Galan Alfonso · lore

Re: [PATCH/RFCv3 2/2] git rebase -i: warn about removed commits

Show 10 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes: 
> > Ideally, you would check the list of commits displayed too. If the 
> > commits sha1 are stable, this should be easy to do. If it's too hard to 
> > test, I'd say its not worth the trouble, but others may disagree. 
> 
> Originally I chose not to check if the SHA-1 were corrects since 
> check_commits was called right after expand_todo_ids and I thought 
> that expand_todo_ids checked them, but from what I understand, it 
> doesn't seem to check if the SHA-1 are commits, I could be wrong 
> though. 

Ignore this email, I completely misunderstood the email I was responding to. (Mailer that doesn't show the quotes by default)

Rémi 
Matthieu Moy· Jun 2, 2015, 14:17 UTC · re: Galan Rémi · lore

Re: [PATCH/RFCv3 1/2] git-rebase -i: add command "drop" to remove a commit

Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
> +test_expect_success 'drop' '

Please, be more descriptive in the first argument of test_expect_success. It's usually a good thing to say not only what the test stresses but also what the expected behavior is.

test_expect_success 'drop actually drops the lines' '
	...
'
?
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Remi Galan Alfonso· Jun 2, 2015, 14:32 UTC · re: Matthieu Moy · lore

Re: [PATCH/RFCv3 1/2] git-rebase -i: add command "drop" to remove a commit

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> : writes
Show 7 quoted lines
> > +test_expect_success 'drop' '
> 
> Please, be more descriptive in the first argument of
> test_expect_success. It's usually a good thing to say not only what the
> test stresses but also what the expected behavior is.
> 
> test_expect_success 'drop actually drops the lines' '

For this one I've based myself on the first argument of some other tests that tests commands for git rebase -i :

> t3404-rebase-interactive.sh:240
> test_expect_success 'squash' '
> t3404-rebase-interactive.sh:679
> test_expect_success 'reword' '
Rémi

← back to recent threads