On Mon, Mar 6, 2023 at 9:24 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote: > On 05/03/2023 05:07, Alex Henrie wrote: > > Suggestions on v5 not incorporated in v6: > > Thanks for adding this, it is really helpful to see what did not change > as well as what did change. It is also helpful to briefly explain why > you disagree with the suggestions so others can understand why you > decided not to make these changes. > > > - Make --rebase-merges without an argument clobber the mode specified in > > rebase.rebaseMerges > > I'm still confused as to why we want the config value to change the > behavior of --rebase-merges. Is it for "git pull --rebase=merges"? If > that's the case then I think a better approach is for pull to parse > rebase.merges and pass the appropriate argument to rebase. That way we > don't break the expectation that command line arguments override config > values. > > > - Remove the test for --rebase-merges=no-rebase-cousins overriding > > rebase.rebaseMerges=rebase-cousins > > - In the tests, check the graph itself instead of checking that the > > graph has not changed by checking that the commit hash has not changed > > I'm not sure what value the existing test has if it only checks that > HEAD is unchanged after the rebase. It could be unchanged because the > rebase fast-forwarded or because it did nothing. Please have a look at the email I sent before sending v6: https://lore.kernel.org/git/CAMMLpeRfD+81fsEtvKFvVepPpwYm0_-AD=QHMwhoc+LtiXpavw@xxxxxxxxxxxxxx/ In that email I tried to explain why I didn't incorporate your three suggestions on v5. Please let me know if it still isn't clear. -Alex
