Re: [PATCH 1/4] revert: report success when using option --strategy
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Jul 15, 2010, 22:37 UTC
- Message-ID
- <20100715223706.GA4788@burratino>
- In-Reply-To
- <20100713232816.7738.17380.chriscool@tuxfamily.org>
Christian Couder wrote:
> "git cherry-pick foo" has always reported success with > "Finished one cherry-pick" but "cherry-pick --strategy" > does not print anything.
[...]
> This patch also refactors the code that prints a message > like "Automatic cherry-pick failed. <help message>". This > code was duplicated in both do_recursive_merge() and > do_pick_commit().
This is good. Nitpicks below.
Show 10 quoted lines
> +++ b/builtin/revert.c > diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh > index f90ed3d..6555f92 100755 > --- a/t/t3508-cherry-pick-many-commits.sh > +++ b/t/t3508-cherry-pick-many-commits.sh > @@ -23,12 +23,36 @@ test_expect_success setup ' > ' > > test_expect_success 'cherry-pick first..fourth works' ' > + cat <<-EOF > expected &&
Style: cat <<-\EOF >expected &&
The \quoting lets reviewers skim through a message without looking for characters that might be expanded. The decreased whitespace is just for consistency.
Show 13 quoted lines
> + Finished one cherry-pick. > + Finished one cherry-pick. > + Finished one cherry-pick. > + EOF > + > + git checkout -f master && > + git reset --hard first && > + test_tick && > + git cherry-pick first..fourth 2>actual && > + git diff --quiet other && > + git diff --quiet HEAD other && > + test_cmp expected actual && > + test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
The --verify here does not accomplish much, since the exit status of git rev-parse is not propagated to test. So strictly speaking it would be more robust to do something like
head=$(git rev-parse --verify HEAD) && fourth=$(git rev-parse --verify fourth) && test "$head" = "$fourth"
though I do not think it is worth changing.
With or without the following changes,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks. ---
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index 6555f92..d90b365 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -23,7 +23,7 @@ test_expect_success setup ' ' test_expect_success 'cherry-pick first..fourth works' ' - cat <<-EOF > expected && + cat <<-\EOF >expected && Finished one cherry-pick. Finished one cherry-pick. Finished one cherry-pick. @@ -40,7 +40,7 @@ test_expect_success 'cherry-pick first..fourth works' ' ' test_expect_success 'cherry-pick --strategy resolve first..fourth works' ' - cat <<-EOF > expected && + cat <<-\EOF >expected && Finished one cherry-pick with strategy resolve. Finished one cherry-pick with strategy resolve. Finished one cherry-pick with strategy resolve.