On Sun, Mar 5, 2023 at 5:22 AM Sergey Organov <sorganov@xxxxxxxxx> wrote: > > Alex Henrie <alexhenrie24@xxxxxxxxx> writes: > > [...] > > > - Rephrase the warning about --rebase-merges="" to recommend > > --rebase-merges without an argument instead, and clarify that that > > will do the same thing > > Not a strong objection to the series as they are, but, provided options > with optional arguments are considered to be a bad practice in general > (unless something had recently changed), I'd like to vote for adding: > > --rebase-merges=on (≡ --reabase-merges="") > > and for suggesting to use that one instead of --rebase-merges="" rather > than naked --rebase-merges. > > It'd be best to actually fix --rebase-merges to always have an argument, > the more so as we have '-r' shortcut, but as this is unlikely to fly due > to backward compatibility considerations, this way we would at least > avoid promoting bad practices further. > > If accepted, please consider to introduce > > --rebase-merges=off (≡ --no-rebase-merges) > > as well for completeness. > > BTW, that's how we settled upon in the implementation of --diff-merges, > so these two will then be mutually consistent. I am curious as to why you say that flags with optional arguments are considered bad practice. I don't see an advantage to adding --rebase-merges=on and then telling users that they ought to always type the extra three characters, even if we were designing the feature from scratch. As it is, we're certainly not going to drop support for --no-rebase-merges or --rebase-merges without an argument, so it seems fine to advertise them to users. And if we do want to add support for passing a boolean value as an argument to --rebase-merges, that can be done after the rebase.rebaseMerges config option is added; it's not a prerequisite. -Alex
