Re: [PATCH 1/2] pull: respect rebase.autostash

[Phil Hord <phil.hord@gmail.com>]

On Fri, Jun 14, 2013 at 8:29 AM, John Keeping <john@keeping.me.uk> wrote:
> On Fri, Jun 14, 2013 at 08:12:56AM -0400, Phil Hord wrote:
>> On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra
>> <artagnon@gmail.com> wrote:
>> > If a rebasing pull is requested, pull unconditionally runs
>> > require_clean_worktree() resulting in:
>> >
>> >   # dirty worktree or index
>> >   $ git pull
>> >   Cannot pull with rebase: Your index contains uncommitted changes.
>> >   Please commit or stash them.
>> >
>> > It does this to inform the user early on that a rebase cannot be run on
>> > a dirty worktree, and that a stash is required.  However,
>> > rr/rebase-autostash lifts this limitation on rebase by providing a way
>> > to automatically stash using the rebase.autostash configuration
>> > variable.  Read this variable in pull, and take advantage of this
>> > feature.
>>
>> This commit message does not tell me what this commit does.  It mostly
>> describes the current situation.  Then it refers to something called
>> "rr/rebase-autostash" which will lose meaning in the future when this
>> commit is no longer current on the list.  A better way to refer to
>> this commit is to say "this commit".  However, even this is not the
>> norm for this project.  The norm here is to avoid such noise by
>> speaking in the imperative mood.  That is, do not tell me what this
>> commit does; instead, tell the code what to do.  See
>> Documentation/SubmittingPatches:
>
> It seems to me that Ram's message is already in the imperative.  The
> only (slight) issue is that rr/rebase-autostash will become hard to find
> once Junio cleans up feature branches that have graduated.  Since that
> branch has graduated to master, it would be clearer to refer to commit
> 5879477 (rebase: implement --[no-]autostash and rebase.autostash,
> 2013-05-12).  Is something like this clearer?
>
>     "git pull" currently cannot be used with the "autostash" feature
>     added to "git rebase" by commit 5879477 (rebase: implement
>     --[no-]autostash and rebase.autostash, 2013-05-12) because it
>     unconditionally calls requre_clean_worktree early on, which results
>     in:
>
>         # dirty worktree or index
>         $ git pull
>         Cannot pull with rebase: Your index contains uncommitted changes.
>         Please commit or stash them.
>
>     Remove this restriction by skipping the call to
>     require_clean_worktree if the "rebase.autostash" configuration
>     variable is set.


Yes, thanks.   I was mislead by my poor understanding all the players
involved.  This disambiguates things nicely.

Phil