git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Mar 25, 2016, 22:29 UTC
Message-ID
<CAPig+cT=UZdueU+sRa1K637nb6FVYhR2z=-SrUsJKnoG+-+Odw@mail.gmail.com>
In-Reply-To
<CA+DCAeRbD3S5Ltse3A6vBcvhKwh9t5av=Fnz98fD2ES5pbAN=Q@mail.gmail.com>
On Fri, Mar 25, 2016 at 2:07 PM, Mehul Jain <mehul.jain2029@gmail.com> wrote:
Show 25 quoted lines
> On Fri, Mar 25, 2016 at 2:01 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> Nit: Some of the test titles spell this as "rebase.autostash" while
>> others use "rebase.autoStash".
>> [...]
>> The title says that this is testing with rebase.autoStash unset,
>> however, the test itself doesn't take any action to ensure that it is
>> indeed unset. As with the two above tests which explicitly set
>> rebase.autoStash, this test should explicitly unset rebase.autoStash
>> to ensure consistent results even if some future change somehow
>> pollutes the configuration globally. Therefore:
>> [...]
>> With the addition of these three new tests, aside from the
>> introductory 'test_{un}config', this exact sequence of commands is now
>> repeated four times in the script. Such repetition suggests that the
>> common code should be moved to a function. For instance:
>
> I agree with all of these comments. I will introduce two new function to
> reduce the code and the above mention loop. Also the work on Matthieu's
> comment.
>
> I feel that most of your comments are necessary and should be there in
> the next patch. But I have a doubt regarding the next patch. As Junio has
> merged v10 of current series in next branch (as noticed from his mail),
> sending a new patch should be based on the current patch (i.e. on next
> branch) or master branch (i.e. continuing with this series)?

I hadn't noticed that v10 was already in 'next'. In this case, the suggested changes should be a new patch series which makes incremental changes to what is already in 'next'. Be sure to mention in the cover letter that the new series should be applied atop mj/pull-rebase-autostash.

Previous: Mehul JainNext: Matthieu Moy
Message 10 of 16 in “introduce --[no-]autostash command line flag”
  1. 0/2 introduce --[no-]autostash command line flagMehul Jain, Mar 21, 2016
  2. 1/2 git-pull.c: introduce git_pull_config()Mehul Jain, Mar 21, 2016
  3. 2/2 pull --rebase: add --[no-]autostash flagMehul Jain, Mar 21, 2016
  4. Matthieu MoyMar 21, 2016
  5. 2/2 pull --rebase: add --[no-]autostash flagMehul Jain, Mar 21, 2016
  6. Eric SunshineMar 25, 2016
  7. Eric SunshineMar 25, 2016
  8. Eric SunshineMar 25, 2016
  9. Mehul JainMar 25, 2016
  10. Eric SunshineMar 25, 2016
  11. Matthieu MoyMar 25, 2016
  12. Mehul JainMar 25, 2016
  13. Matthieu MoyMar 25, 2016
  14. Mehul JainMar 25, 2016
  15. Eric SunshineMar 25, 2016
  16. Mehul JainMar 25, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.