Re: [PATCH] docs: maintainer: discourage taking conversations off-list

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

 



Em Fri, 12 Jul 2024 16:45:04 -0700
Jakub Kicinski <kuba@xxxxxxxxxx> escreveu:

> On Fri, 12 Jul 2024 20:11:56 +0200 Mauro Carvalho Chehab wrote:
> > Not sure what this somewhat obscure message wants to accomplish.
> > 
> > It is quite common to have developers and maintainers discussing 
> > outside public forums and internally at the companies they're working 
> > for. There are lots of reasonable exceptions besides security. On my
> > years of experience, the reasons I've seen more often are:
> > 
> > 1. language and/or cultural barriers;
> > 2. teaching and mentoring new developers to start contributing upstream;
> > 3. need to have internal discussions in the light of some IP protected
> >    material.
> > 
> > (1) and (2) are very common for non-native English speakers
> > and for newbies, and we do want to have more contributions from
> > them. (3) is unavoidable, as discussions related to protected
> > IP can't be disclosed due to legal reasons.  
> 
> Those are valid points but I don't know how to weave them in without
> losing clarity.
>
> The goal is also to call out such behavior to
> _contributors_, 

Then, placing it under Documentation/maintainer is not the right
place ;-)

> so that they know they can reach out to more senior
> maintainers if it happens to them. We don't know when discussion is
> taken private (by definition). Otherwise contributors get mistreated 
> by some corpo-maintainer and they turn away from Linux :(
> 
> > Also, if you take it to the letter, have discussions on LPC, 
> > summits BoFs, other events handled by the open source community 
> > and wall conversations are "closed forums/private conversations".
> > I've seen a lot of good results produced on such private
> > conversations that solved relevant conflicts and got
> > materialized as great contributions.
> > 
> > There's nothing wrong with that, provided that the outcoming of
> > such internal discussions are reflected publicly as patches,
> > summit minutes, LWN articles, etc.  
> 
> Would it help if we speak of "open forums" instead of mailing list?
> I think LPC including "hallway track" fall squarely under "conducted 
> in a manner typical for the larger subsystem." Here's slightly edited 
> version:

Well, hallway track, invitation-only events and such aren't exactly
"open forums".

> 
>   Open development
>   ----------------

Assuming that this got moved to a contributor's document, as from your
comments the target audience is ocasional community contributors,
see my comments below.

> 
>   Discussions about user reported issues, and development of new code
>   should be conducted in a manner typical for the larger subsystem.

This seems to vague and meaningless to an occasional community
developer.

For instance, how someone that sends a contribution to subsystem X 
knows if the subsystem is a "larger subsystem", so it is "typical"?
What's the "typical" rules?

Btw, larger subsystems usually have their own set of rules. A couple
of them are documented at maintainer-profile.rst, but most don't.

>   It is common for development within a single company to be conducted
>   behind closed doors.

IMO, this is somewhat out of scope. I mean, what a contributor should
expect from this?

I bet a new contributor to a driver made by company X, after reading
this paragraph, would try to submit patches privately to company X
maintainers, which seems to be the opposite from the message you
wanted to give.

>   However, development and discussions initiated
>   by community members must not be redirected from public to closed forums
>   or to private email conversations. Reasonable exceptions to this guidance
>   include discussions about security related issues.

In the light of a contributor focused document, I would be a lot
more direct here, using a text similar to this:

	Please don't send patches or reply privately to discussions
	initiated on public forums, as most maintainers won't appreciate
	such kind of behavior. Private communications outside company's
	own internal discussions are usually tolerated only when there 
	are very good reasons for that, like when talking about
	undisclosed security issues.

> 
> > The only issues I see with such talks is that the work when
> > co-authored should be properly marked as such and that 
> > reviewews/acks taken behind doors don't have the same meaning
> > as an upstream review, as they may be due to some internal 
> > formalities.
> > 
> > IMO, the best would instead to give a positive message. E. g.
> > something like:
> > 
> > 	Maintainers must encourage discussions and reviews to happen
> > 	at public mailing lists, avoiding whenever possible to have
> > 	internal discussions.  
> 
> That's not the message, tho. If someone emails a company privately 
> that's fine. If company has internal processes for its development -
> also fine (as explicitly called out). I'm trying to set the baseline,
> not describe the ideal world.
> 
> I am specifically calling out that if someone submits a patch, or
> reports a regression the correct response is to review it on the list.
> Like a normal person.

It is clear now, but as Dan pointed out, this is the wrong document
for contributors, as Documentation/maintainers are focused on
maintainers ;-)

So, if something has to be added there, it should be to try to
setup a baseline for maintainer's typical behavior.

> Not reply privately that "it's on the company roadmap, just wait" :|
> Or reply with a patch company has "forgotten to upstream"..

I'm afraid that, in the first case, we'll still see this privately,
as companies won't be disclosing publicly what it is on their
roadmaps.

"Forgotten" patches should indeed be sent publicly for review.
The text I added earlier should hopefully cover it.

Perhaps a note about that at the beginning of
Documentation/process/submitting-patches.rst could be more
effective, e. g. something like this:

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 66029999b587..3bdfb820a707 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -16,6 +16,9 @@ for a list of items to check before submitting code.
 For device tree binding patches, read
 Documentation/devicetree/bindings/submitting-patches.rst.
 
+Please notice that Linux patches are meant to be submitted publicly.
+Don't submit patches privately, except for security patches.
+
 This documentation assumes that you're using ``git`` to prepare your patches.
 If you're unfamiliar with ``git``, you would be well-advised to learn how to
 use it, it will make your life as a kernel developer and in general much

> Maybe it's a cultural thing, but to me this is where the relentless
> positivity is counter-productive. I don't want to encourage people
> to be angles. I want them not to do the shitty thing.

Again, you placed the comments at the maintainer's document, where
the scope of of "don't do this" is too limited. I would expect
that people that volunteered themselves to do some maintainership 
are more willing to react to positive messages about the expected
maintainer's behavior.

On a contributor's document, though, I agree with you that a set
of rules like don't do top posting, don't submit patches privately,
and such are more effective.

Thanks,
Mauro




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux