On Fri, Mar 18, 2016 at 9:54 AM, Eric Sunshine <sunsh...@sunshineco.com> 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. >> +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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
