Re: [PATCH 4/8] SubmittingPatches: update extra tags list

From: Josh Soref <jsoref@gmail.com>
Date: Dec 20, 2023, 16:09 UTC
Message-ID: <CACZqfqAiSpGP5ABN7MZ50Za=-vAEKnqE0ravzEMbLJCByfRd8w@mail.gmail.com>
In-Reply-To: <CABPp-BH_iP2KjPi-5kW8ROQWfy8XoUmbGhyT-Y1dZGtCtZXQGQ@mail.gmail.com>

On Wed, Dec 20, 2023 at 10:30 AM Elijah Newren <newren@gmail.com> wrote:

Show 8 quoted lines

> On Wed, Dec 20, 2023 at 7:18 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> > Thanks for adding these, they are widely used and should be documented.
> > The patch also adds a mention for "Noticed-by:" - I'm less convinced by
> > the need for that as I explain below.
> >
> > > Updating the create suggestion to something less commonly used.
> >
> > I'm not quite sure I understand what you mean by this sentence.

Tentatively rewritten as:
    Updating the "create your own tag" suggestion as 'Mentored-by' has been
    promoted.

This commit is adding bulleted items including promoting 'Mentored-by', which means that the suggestion of "invent your own" would really need a new suggestion.

Personally I'm not a fan of "invent your own", but I'm trying to follow "when in Rome" (which is a big thing in the Git process documentation covered by the two files subject to this series). More on this later.

Show 23 quoted lines

> > > diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> > > index 32e90238777..694a7bafb68 100644
> > > @@ -348,6 +348,8 @@ If you like, you can put extra tags at the end:
> > >
> > >   . `Reported-by:` is used to credit someone who found the bug that
> > >     the patch attempts to fix.
> > > +. `Noticed-by:` liked `Reported-by:` indicates someone who noticed
> > > +  the item being fixed.
> >
> > I wonder if we really need a separate "Noticed-by" footer in addition to
> > "Reported-by". Personally I use the latter to acknowledge the person who
> > reported the issue being fix regards of weather I'm fixing a bug or some
> > other issue.
>
> I'm not sure I'd mention both either; feels like we're making it hard
> for users to choose.  Also, I think there's a minor distinction
> between them, but it's hard to convey simply: To me, Reported-by
> suggests someone sent a mail to the list specifically about the bug or
> issue in question.  Also, to me, Noticed-by suggests that during a
> back-and-forth discussion of some sort on another topic, a fellow Git
> contributor noticed an issue and mentioned it as an aside.  But,
> that's how _I_ would have used them, I didn't do any digging to find
> out if that's really how they are used.

Given that Noticed-by is more common than Co-authored-by, I have a hard time arguing that it shouldn't be included. You could see that I struggled with it based on my lousy first drafts [1].

Anyway, tentatively:

. `Noticed-by:` like `Reported-by:` indicates a Git contributor who called the item (being fixed) out as an aside.

Here, I'm struggling with the tension between "noticed-by probably hints that something is being fixed" and "noticed-by is addressing who suggested it and why we're attributing it to them"

"as an aside" is itself an ellipsis for something like "as an aside to some unrelated discussion and didn't really circle back to it as a top level discussion point, but here we're closing the loop" -- but this is obviously way too long-winded to be the written form.

Show 6 quoted lines

> Either way, if we're going to define them as synonyms, I'd rather we
> just left the less common one out.  If there's a distinction, and it's
> not a pain to describe, then maybe it'd be worth adding both.
>
> If we do add both, though, we at least need to fix "liked" to "like"
> in your description.

Right, it's a "first draft" [1]...

Show 19 quoted lines

> > > +. `Co-authored-by:` is used to indicate that multiple people
> > > +  contributed to the work of a patch.
> >
> > Junio's comments in [1] may be helpful here
> >
> >      If co-authors closely worked together (possibly but not necessarily
> >      outside the public view), exchanging drafts and agreeing on the
> >      final version before sending it to the list, by one approving the
> >      other's final draft, Co-authored-by may be appropriate.
> >
> >      I would prefer to see more use of Helped-by when suggestions for
> >      improvements were made, possibly but not necessarily in a concrete
> >      "squashable patch" form, the original author accepted before
> >      sending the new version out, and the party who made suggestions saw
> >      the updated version at the same time as the general public.
> >
> > So I think we should be clear that "Co-authored-by:" means that multiple
> > authors worked closely together on the patch, rather than just
> > contributed to it.

Tentatively: . `Co-authored-by:` is used to indicate that people exchanged drafts of a patch before submitting it.

Note that I'm dropping `multiple` -- people implies that.

. `Helped-by:` is used to credit someone who suggested ideas for changes without providing the precise changes in patch form.

Show 5 quoted lines

> > I think we use "Montored-by:" specifically to credit the mentor on
> > patches written by GSoC or Outreachy interns and "Helped-by:" for the
> > general case of someone helping the patch author.
>
> Yes; I'd like for these to be distinguished in this way or something similar.

. `Mentored-by:` is used to credit someone with helping develop a patch as part of a mentorship program (e.g., GSoC or Outreachy).

Show 5 quoted lines

> > >   You can also create your own tag or use one that's in common usage
> > > -such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
> > > +such as "Thanks-to:", "Based-on-patch-by:", or "Improved-by:".
> >
> > What's the difference between "Improved-by:" and "Helped-by:"?

I dunno. This entire section (as I called out at the beginning) is frustrating...

Anyway, see all of us struggle with it below:

> > In
> > general I'd prefer for us not to encourage new trailers where we already
> > have a suitable one in use.

If this text needs to survive without being drastically changed, it needs a warning saying "don't create a new tag if there's an already used tag that seems like it'll cover what you're trying to say -- here's how to review them...". Note that I'd argue the cost being asked of someone to do this research far exceeds what is reasonable to ask of a new contributor, which is why I'd rather say that new contributors shouldn't be inventing tags.

> Agreed.

I personally agree. I think encouraging non-core contributors to invent their own is not a good idea. It leads to various things (including inconsistently cased items because users fail to review the current set / understand them/their-construction).

Saying that it's ok for core contributors to suggest a new tag and that if a core contributor suggests a new tag that the person writing the current series should just accept the tag and trust that it'll be ok.

Note: I'm not going to draft wording to this effect on my own, and if
I were to provide such a change, it'd be its own commit prior to the
one here, because it's a significant process change as opposed to
clarifying the list of recommended tags.

> > Thanks for working on this, it will be good to have better descriptions
> > of our common trailers.
>
> Agreed here as well; thanks for the work, Josh.

[1] The Late Show With Stephen Colbert has a running segment called First Drafts which showcases lousy first drafts of greeting cards, they've https://www.cbsstore.com/products/the-late-show-with-stephen-colbert-first-drafts-greeting-card-pack-sc1193