Re: [PATCH v2 1/2] rebase: support non-interactive autosquash

[Andy Koppe <andy.koppe@gmail.com>]

On 06/11/2023 00:50, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> 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