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

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

 



On Wed, Dec 20, 2023 at 10:30 AM Elijah Newren <newren@xxxxxxxxx> wrote:
> On Wed, Dec 20, 2023 at 7:18 AM Phillip Wood <phillip.wood123@xxxxxxxxx> 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.

> > > 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.

> 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]...

> > > +. `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.

> > 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).

> > >   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





[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