Re: [PATCH] cherry-pick: allow "-" as abbreviation of '@{-1}'

[Thomas Rast <trast@inf.ethz.ch>]

Hiroshige Umino <hiroshige88@gmail.com> writes:

> As "git cherry-pick -" or "git merge -" is convenient to
> switch back to or merge the previous branch,
> "git cherry-pick -" is abbreviation of "git cherry-pick @{-1}"
> to pick up a commit from the previous branch conveniently.

The first line is confusing.  Did you mean to invoke the existing 'git
*checkout* -' and 'git merge -' functionality as a reason why 'git
cherry-pick -' should exist?

What other commands could reasonably use the '-' shorthand?

[...]
> diff --git a/t/t3512-cherry-pick-last.sh b/t/t3512-cherry-pick-last.sh

Do you have to use a new test file for this?

[...]
> +test_expect_success 'setup' '
> + echo hello >world &&
> + git add world &&
(*)
> + git commit -m initial &&
> + git branch other &&
> + echo "hello again" >>world &&
> + git add world &&
(*)
> + git commit -m second
> +'

Our style is to indent the test snippets with a hard tab, not a single
(or eight, for that matter) space.

[...]
> +test_expect_success 'cherry-pick the commit in the previous branch' '
> + prev=$(git rev-parse HEAD) &&
> + git checkout other &&
(*)
> + git cherry-pick - &&
> + test "z$(git rev-parse HEAD)" = "z$prev"
> +'

If you insert 'test_tick' in the places marked with (*), the test fails.

The tests run under a fake clock to ensure that everything, including
the SHA1s produced, are deterministic.  You never advance the clock, so
all commits generated in this script share the same timestamp.

This means that the cherry-pick of 'second' has the same SHA1 as the
original: its tree, parents, author, timestamp etc. all agree.  If you
advance the clock at the last (*), this fails.  You should find some
other way of checking what was picked, e.g., by looking at the file
contents.

That said, please use test_commit in the 'setup' snippet instead of
manually rolling the commits.  It will lead to shorter code, and it
handles test_tick for you.  It is documented in t/README and in a
comment in t/test-lib-functions.sh.  (You still need test_tick
immediately before the cherry-pick!)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch