From: Andy Koppe Date: Sat, 11 Nov 2023 14:26:52 GMT Subject: Re: [PATCH v2 1/2] rebase: support non-interactive autosquash Message-ID: <307994aa-6a3d-41b8-86cb-c891abc13e52@gmail.com> In-Reply-To: On 06/11/2023 00:50, Junio C Hamano wrote: > Andy Koppe writes: > Our log > message convention is to first describe what happens in the system > in the present tense to illustrate why it is suboptimal, to prepare > readers' minds to anticipate the solution, which is described next. > > When asking reviews on a new iteration [PATCH v(N+1)], please > summarize the differences relative to [PATCH vN]. For explaining > such incremental changes for individual patches, here between the > three-dash line and the diffstat is the place to do so. When you > have a cover letter [PATCH 0/X], it can be done in that messaage. > Either way is OK. Doing both is also helpful as long as the > explanation done in two places do not contradict with each other. > If you tried to format the documentation before sending this patch, > you'd have seen the second paragraph formatted as if it were a code > snippet. Dedent the second paragraph (and later ones if you had > more than one extra paragraphs), and turn the blank line between the > paragraphs into a line with "+" (and nothing else) on it. See the > description of `--autosquash` option in Documentation/git-rebase.txt > for an example. Sad thing is that I knew most of that from reading the contribution guidelines and previous experience, but obviously I don't always remember. So thanks for you patience in re-explaining that. > OK, by clearing opts->config_autosquash in this function, you keep > the rebase.autosquash to be "the last one wins" as a whole. If a > configuration file with lower precedence (e.g., /etc/gitconfig) says > "[rebase] autosquash" to set it to "interactive,no-interactive", a > separate setting in your ~/.gitconfig "[rebase] autosquash = false" > would override both bits. > > A more involved design may let the users override these bits > independently by allowing something like "!no-i" (take whatever the > lower precedence configuration file says for the interactive case, > but disable autosquash when running a non-interactive rebase) as the > value, but I think the approach taken by this patch to allow replacing > as a whole is OK. It is simpler to explain. > > Giving short-hands for often used command line options is one thing, > but I do not think a short-hand is warranted here, especially when > the other one needs to be a less-than-half legible "no-i" that does > not allow "no-int" and friends, for configuration variable values. > I'd strongly suggest dropping them. Dropped in v4, along with the attempt to expand rebase.autoSquash, following Phillip's review. Regards, Andy