Re: [PATCH v9 2/2] pull --rebase: add --[no-]autostash flag

On Fri, Mar 18, 2016 at 9:54 AM, Eric Sunshine [off-list ref] wrote:

Since this is now a patch series rather than a single patch, another
way to help reviewers is to use a cover letter (see git-format-patch
--cover-letter) where you'd explain the changes, and, importantly,
include an interdiff between the previous and current versions.

Point noted for the future patches.

quoted

+test_expect_success 'pull --rebase: --autostash overrides rebase.autostash' '

Why do titles of some of the new test titles have a ":" after "rebase"
while other don't?

Also, how about normalizing the titles so that the reader knows in
which tests rebase.autostash is 'true' and in which it is 'false'?
Presently, it's difficult to decipher what's being tested based only
on the titles.

If it's so then how about the tests titles to be the following:

* pull --rebase: --autostash works with rebase.autoStash set true

* pull --rebase: --autostash works with rebase.autoStash set false

* pull --rebase: --no-autostash works with rebase.autoStash set true

* pull --rebase: --no-autostash works with rebase.autoStash set false

Earlier I tried to keep it as less verbose as possible (and probably
made it hard to decipher). Does the above titles seems short and
informative to you? If so then I will use them instead of earlier ones.

Finally, shouldn't you also be testing --autostash and --no-autostash
when rebase.autostash is not set?

If rebase.autoStash is not set then config.autostash will remain zero
through out the process. What I want to point out is that rebase.autoStash
, if not set, is equivalent to being set false. So adding tests regarding
"--[no-]autostash with rebase.autoStash unset" seems equivalent to tests
" pull --rebase: --autostash works with rebase.autoStash set false" and
"pull --rebase: --no-autostash works with rebase.autoStash set false".

Thanks,
Mehul