Re: [PATCH 2/2] rebase -i: new option --name-rev
- From
Thomas Rast <trast@inf.ethz.ch>
- Date
- Mar 8, 2012, 10:56 UTC
- Message-ID
- <87399jnyxh.fsf@thomas.inf.ethz.ch>
- In-Reply-To
- <1331203358-28277-2-git-send-email-domq@google.com>
On a general note: you are submitting a completely new feature touching a heavily-used tool (and code path) during -rc0 time. As a rule of thumb: Don't do that. If you do it, don't Cc Junio unless it's his area of code.
Dominique Quatravaux <domq@google.com> writes:
> If set, the second column of the rebase todo contains named revisions (obtained > with git name-rev) instead of short SHA1s.
Hum. I'm not sure yet if I find that very useful, since frequently the names will just be 'topic', 'topic~1', ...., 'topic~N' if you are rebasing a topic with N+1 commits not in master. But you might, so who am I to judge.
> +--name-rev:: > +--no-name-rev::
The --no- version is implicitly always supported, see gitcli(1).
> +configuration variable `rebase.interactivenamerev`, this option can be
You should spell it in a more readable way such as rebase.interactiveNameRev. The config machinery internally downcases everything so the cosmetics won't prevent it from working.
Show 7 quoted lines
> - shortsha1=$(echo $sha1 | cut -c1-7) > + if test t = "$name_rev" > + then > + rev="$(git name-rev $sha1 | cut -d\ -f2)" > + else > + rev=$(echo $sha1 | cut -c1-7) > + fi
In the spirit of your previous patch, wouldn't it be faster to run 'git name-rev --stdin' within the pipeline?
How does this interact with --autosquash?
Show 5 quoted lines
> +test_expect_success 'Symbolic revisions in --name-rev' ' > + exec > debug.log 2>&1 && > + FAKE_LINES="exec_cp_.git/rebase-merge/git-rebase-todo_rebase-todo 1 2" git rebase -i --name-rev HEAD~2 && > + test_cmp expect-rebase-todo rebase-todo > +'
In line with the --autosquash concern, please write a test that uses both option (and verifies that *both* work!).
--
Thomas Rast
trast@{inf,student}.ethz.ch