Re: [PATCH 0/2] contrib/subtree: Add -S/--gpg-sign option

From: Patrik Weiskircher <patrik.weiskircher@nutrient.io>
Date: May 30, 2025, 14:33 UTC
Message-ID: <CANMzfzhhHX1J_vPH-DMD2kPmQ6G_6YUV=7aZ5JojGy-LU6guAQ@mail.gmail.com>
In-Reply-To: <xmqqfrgnhuuw.fsf@gitster.g>

On Thu, May 29, 2025 at 7:15 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

>
> Patrik Weiskircher <patrik@pspdfkit.com> writes:
>
> > Hi!
> >
> > We use git subtree a lot to manage our dependencies, but recently started
> > requiring signed commits. This patch adds support for signing commits to
> > git subtree.
> >
> > This is my first submission to the Git project - you don't have to be gentle,
> > but please let me know if I can improve anything.

Apologies, Junio. I replied only to you and with HTML emails. I'm very new here and I'm very much used to different workflows. I'll be better. Please disregard my previous emails.

Show 8 quoted lines

>
> A few things ;-) starting from the log message where you didn't
> quite add why we would want to do these changes.
>
> Also, I think these two should be combined into a single patch.  We
> add a feature and make sure the feature works correctly and we also
> make sure that the feature does not misfire when the user does not
> ask it to trigger, all in the same commit.

I looked at your review, and I agree with everything you said. My patch wasn't very well thought out.

I'm also at a crossroads here. I'm not sure if the optional argument parsing will work out nicely in bash. Let me explain:

git-subtree uses `git rev-parse --parseopt`. This *does* have support for optional arguments, but highly recommends using `--stuck-long`, otherwise unambiguously parsing arguments becomes impossible.

This is true. There's no way to differentiate between the optional argument or the next positional parameter. Therefore, I would have to use `--stuck-long`.

This in itself isn't overly difficult. There's only a couple of parameters in the script that that would affect. The problem I have with it is that I don't see a single user of `--stuck-long` in the whole repo. I don't necessarily want to become the first one. Seems like something destined to be cleaned up at some point.

This means my options become:
1. use --stuck-long or
2. make -S not take any parameter or
3. make -S require a parameter

I'm not happy with any of these options, but considering I really would like to make this work, I would probably go with 1.

Any opinions on this?

Thanks! Patrik