Re: [PATCH] Documentation: suggest "reset --keep" to undo a commit

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 21, 2011, 17:34 UTC
Message-ID: <7voc7ap3dp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110121073730.GA26276@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 6 quoted lines

> When one's only goal is to move from one commit to another, reset
> --keep is simply better than reset --hard, since it preserves local
> changes in the index and worktree when easy and errors out without
> doing anything when not.  Update the two "how to remove commits"
> examples in this vein.  "reset --hard" is still explained in a later
> example about cleaning up during a merge.

I agree with the first sentence but I do not think its conclusion would lead to changes to "how to _remove_ commits". The examples were written in contexts (explanatory text <$n>) where hard makes sense, and the context needs tweaking to make keep makes more sense than hard does.

For example, the first one's original sequence is this:

    $ git branch topic/wip
    $ git reset --hard HEAD~3
    $ git checkout topic/wip

The text explains the motivation behind these series of commands in <1> but it has one untold assumption behind it; the user did the review to reach the conclusion that the recent changes are premature after fully committing (i.e. the working tree is clean). That is why "hard" worked just fine.

But the user could do the reviewing and thinking with some local changes still in the working tree (they are incredients for the fourth commit yet to be made) and decide to branch at that point. The description in <1> needs to be updated to hint that there can be uncommitted changes, e.g.

	You have worked for some time, made a few commits, and may have
	uncommitted changes.  After reviewing the current state, you
	realized that ...

Using --keep may help the user do so, but only if the local changes do not conflict with the changes in the recent commits to be discarded, right?

    Side note: Regardless of any of the above, the section header needs to
    be updated---it is not "Undo *A* commit", we are excluding three from
    the current branch.

By the way, a more natural way to do this would actually be:

    $ git checkout -b topic/wip
    $ git branch -f @{-1} HEAD~3

or using the stash:

    $ git stash ;# save local changes
    $ git branch topic/wip ;# and mark the tip before rewinding
    $ git reset --hard HEAD~3 ;# you could say --keep here too
    $ git checkout topic/wip ;# and then continue
    $ git stash pop ;# with the local changes

The branch/reset/checkout sequence itself, either with hard or keep, looks more like a contrived example to show "reset" than the best way to solve the problem in the scenario presented there. Probably we would want to drop this one altogether, or keep the scenario and explain the best solution in somewhere else (e.g. tutorial).

Show 9 quoted lines

> @@ -163,7 +163,7 @@ Undo commits permanently::
>  +
>  ------------
>  $ git commit ...
> -$ git reset --hard HEAD~3   <1>
> +$ git reset --keep HEAD~3   <1>
>  ------------
>  +
>  <1> The last three commits (HEAD, HEAD^, and HEAD~2) were bad

Please tell a story where keep makes more sense than hard by enhancing the explanatory text <1> associated with this section. The current text says that the three topmost commit representing what you have recently worked so far are all unwanted, strongly hinting that hard is more appropriate thing to do than keep, which is not what we want if we are changing the example to use keep.

It would be sufficient to just hint that the uncommitted changes that you have in your working tree are unrelated to what these three commits wanted to do (e.g. you always keep small changes around, such as debugging printf's and a change to the version string in Makefile---you do not intend to commit them and they are unrelated to the commits you are discarding), and you do want to keep them around if you can.