All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Nakul V S via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>,
	Nakul V S <vsnakul@gmail.com>
Subject: Re: [PATCH] sequencer: refuse to revert shallow boundary commits by default
Date: Mon, 26 Oct 2026 10:58:44 +0000	[thread overview]
Message-ID: <235e7dcd-2d29-47e4-b68f-9b02f80085dd@gmail.com> (raw)
In-Reply-To: <xmqqqzhina26.fsf@gitster.g>

On 21/10/2026 20:11, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>> On 21/10/2026 11:09, Nakul V S via GitGitGadget wrote:
>>
>> Thanks for working on this, it is a useful and timely improvement as
>> there was some discussion about improving the behavior of git with
>> regard to shallow boundaries at the recent git merge conference. I
>> wonder if it would be better to refuse to cherry-pick and rebase a
>> shallow boundary commit as well. Do you know of a sensible use for
>> reverting a shallow boundary or is the new option added out of caution
>> in case someone wants it?
> 
> Very good point.
> 
> The next natural question is where the modification should go if we
> want to make sure we protect these shallow boundaries not just from
> revert but also from "cherry-pick", "rebase", and possibly "commit
> --amend".  Is sequencer.c:do_pick_commit() the right place to cover
> all of them?  If not, would it make sense to have a helper function
> that different code paths to protect shallow boundaries from these
> operations can call, and add the first call to that helper funcion
> to the location this patch adds new code to?

 From the user's point of view, I think do_pick_commit() is a bit late 
to error out. It would be nicer to refuse to add a shallow commit to the 
todo list in the first place which means we'd need a helper function 
that's called from sequencer_make_script(), make_script_with_merges(), 
single_pick() and sequencer_pick_revisions(). That is a bit of a pain 
but would make for a better UI.

Thanks

Phillip


      reply	other threads:[~2026-10-26 10:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 10:09 [PATCH] sequencer: refuse to revert shallow boundary commits by default Nakul V S via GitGitGadget
2026-10-21 15:20 ` Phillip Wood
2026-10-21 19:11   ` Junio C Hamano
2026-10-26 10:58     ` Phillip Wood [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=235e7dcd-2d29-47e4-b68f-9b02f80085dd@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --cc=vsnakul@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.