Re: [PATCH v2] sequencer: refuse to pick or revert shallow boundary commits

[Junio C Hamano <gitster@pobox.com>]

"Nakul V S via GitGitGadget" <gitgitgadget@gmail.com> writes:

>     Changes since v1:
>     
>      * Also refuse cherry-pick and rebase (merge, interactive and --apply
>        backends), as suggested by Phillip.
>      * Check when the list of commits is built rather than in
>        do_pick_commit(), so a refused command leaves no state behind. Uses a
>        single shared helper.
>      * --allow-shallow-boundary is now accepted by cherry-pick as well, and
>        is saved across --continue.
>      * Dropped the option from the revert synopsis.
>      * rebase --autostash restores the user's changes if the rebase is
>        refused.

All of these sounds sensible.

>      * Rebased onto current master.

Was it necessary?  On which commit was the previous round based?
Has there been any changes that made it necessary to rebase since
then?

>     Not covered here: git commit --amend, and pick/revert lines typed into
>     the rebase todo by hand.

It is OK to start small and extend coverage step by step, as we are
making strict improvements.  Do we however have some vision on how
these would later be supported?

> +	else if (!strcmp(key, "options.allow-shallow-boundary"))
> +		opts->allow_shallow_boundary =
> +			git_config_bool_or_int(key, value, ctx->kvi, &error_flag);

Hmph, why not git_config_bool()?

Unlike commit.verbose or status.submodulesummary, I didn't expect
this to have anything more than a simple "yes we allow it" or "no
don't allow it" answer.  "all the other boolean neighbors somehow
use it" is an understandable answer, but I expect people to do
better than that.

Overall things seem to be in a good shape.  Except for tiny details
like this one.

Thanks.