This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch review by coworkers


On 09/27/2017 06:00 AM, Siddhesh Poyarekar wrote:
> On Wednesday 27 September 2017 04:45 PM, Florian Weimer wrote:
>> Red Hat now has several people who regularly work on upstream glibc.
>> This raises questions regarding patch review.
>>
>>
>> Does the community expect that we Red Hatters deal with patches from
>> posting to commit, having colleagues doing the review etc.?  The
>> argument in favor of that is that if something interests Red Hat, Red
>> Hat should throw resources at it, and not rely on the community to do
>> the work.
>>
>> -or-
>>
>> Does the community expect that we avoid patch review by coworkers?  The
>> argument for that is that it avoids any incentive to game the review
>> system, and it puts community interests first.
>>
>>
>> Note that I'm talking about the ordinary day-to-day work here, not
>> patches we know or have reason to suspect to be controversial.  And if
>> we err, we can still revert patches, of course.
> 
> I use the following priority order for patch reviews (subject to time
> availability of course):
> 
> 1. Modules I maintain (benchtests, tunables, etc.)
> 2. Modules I am actively developing or interested in (aarch64, etc.)
> 3. Patches I am specifically pinged for, on list or off it
> 4. Oldest first in modules I am capable of reviewing
> 
> I would expect something similar from others too and prioritizing
> colleagues' patches is probably fair game under point (2), so is
> prioritizing patches for friends and commercial partners who make
> requests offline to prioritize patches.  I'm just happy that there are
> so many people working on glibc finally and as long as they review
> patches of non-Red Hatters regularly enough (and I have had no reason to
> complain so far) I think this is really fine.

I wear 2 hats.

* GNU Maintainer (Steward).

* Red Hat Employee.

I try hard to balance those two roles.

However, when I have a Red hat on, I review Red Hat patches, and that's
a natural consequence of what I'm interested in.

I also advance, review, and champion other things when I have my FSF
hat on.

Like all things in life it's a balance.

Therefore I think you *can* review other Red Hatters patches without
the fear of collusion, so long as your objective and professional
(and who isn't right?).

I think I was called out on this once by Roland when I reviewed what
I thought was a good patch posted by Siddhesh for default thread
stack sizes. I disagreed with Roland, since all we were doing was
adding an env var which we could delete later IMO, and had no API/ABI
impact. However, the subsequent review by Roland and Siddhesh result
in a useful new API extension for setting the default thread stack
size for all future threads. So the moral of the story is: get involved
and help review :-)

-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]