Re: [PATCH v4 8/9] rebase -i: teach --autosquash to work with amend!

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2 Feb 2021 at 08:50, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>
> On Fri, Jan 29, 2021 at 1:25 PM Charvi Mendiratta <charvi077@xxxxxxxxx> wrote:
> > If the commit subject starts with "amend!" then rearrange it like a
> > "fixup!" commit and replace `pick` command with `fixup -C` command,
> > which is used to fixup up the content if any and replaces the original
> > commit message with amend! commit's message.
> >
> > Signed-off-by: Charvi Mendiratta <charvi077@xxxxxxxxx>
> > ---
> >  sequencer.c                     | 23 ++++++++++++++++-------
> >  t/t3437-rebase-fixup-options.sh | 12 ++++++++++++
> >  2 files changed, 28 insertions(+), 7 deletions(-)
>
> Is this new behavior of recognizing "amend!" in the subject documented
> anywhere? I checked the documentation patch [9/9] but didn't see any
> mention of it.
>

No this patch series does not include that but included in the patch series of
amend! commit implementation that is not sent to the mailing list yet.
(Apology for the confusion)

> > diff --git a/sequencer.c b/sequencer.c
> > @@ -5662,6 +5662,12 @@ static int subject2item_cmp(const void *fndata,
> > +static inline int skip_fixup_amend_squash(const char *subject, const char **p) {
> > +       return skip_prefix(subject, "fixup! ", p) ||
> > +              skip_prefix(subject, "amend! ", p) ||
> > +              skip_prefix(subject, "squash! ", p);
> > +}
>
> While the function name skip_fixup_amend_squash() may be accurate, it
> won't scale well. What happens when additional fixup-like prefixes are
> added in the future? Does the function name get extended to name them,
> as well? How about choosing a more generic, yet still meaningful,
> function name which doesn't suffer from this scaling problem. Perhaps
> skip_fixupish() or fix_squashlike() or something.
>

I agree completely, and will change the function name  to skip_fixupish().

> Also, making this function `inline` seems like a case of premature optimization.

Okay, will remove `inline`.

Thanks and Regards,
Charvi



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux