[Pulp-dev] Pulp Code Owners

Milan Kovacik mkovacik at redhat.com
Tue Aug 14 13:56:30 UTC 2018


On Tue, Aug 14, 2018 at 3:47 PM, David Davis <daviddavis at redhat.com> wrote:

> On Tue, Aug 14, 2018 at 9:35 AM Milan Kovacik <mkovacik at redhat.com> wrote:
>
>>
>>
>> On Tue, Aug 14, 2018 at 1:26 PM, David Davis <daviddavis at redhat.com>
>> wrote:
>>
>>> The relevant party could either be a subset of the commit bit owners
>>> (e.g. task group) or a set of people who don’t have the commit bit (e.g.
>>> QE). See the team examples from my original email.
>>>
>>
>>  So what you mean is actually a trusted subset of commit bit owners, like
>> the SMEs?
>>
>
> These teams aren’t necessarily a subset of commit bit owners but yes
> they’ll be subject matter experts (SMEs) for the code they own. Take QE for
> example. They might not have the commit bit to the pulp repo but they are
> still the SMEs for pulp_smash tests and thus they’ll probably be code
> owners for the smash tests in pulp and pulp_file.
>
>
>>  So we don't trust all commit bit owners equally when it comes to
>> particular git (sub)tree?
>>
>  And we trust (by blocking the merge) on e.g QE approving a PR more than
>> the commit bit owner that is outside of the subset?
>>  Or is it rather about decoupling the code review duty from the commit
>> bit ownership?
>>
>
> To answer these questions, I don’t think it’s about trust. It’s about (as
> you mention) decoupling merging code from code reviews. We want to make
> sure the appropriate people get notified and have a chance to review the
> PRs for which they are SMEs.
>

This has the same issue as the commit bit ownership, it's just more fine
grained and bound to particular subtrees: how does one become an SME?
What's the SME lifecycle?


>
>
>>  Why do we need commit bit owners then?
>>
>
> How else do we merge the code if no one has a commit bit?
>

  thru a bot for instance


>
>
>>
>> Cheers,
>> milan
>>
>>
>>>
>>> Daniel, you are correct. The only caveat is that PRs can’t be merged if
>>> they touch a file owned by a team and haven’t been approved by that team.
>>>
>>> David
>>>
>>>
>>> On Tue, Aug 14, 2018 at 6:35 AM Milan Kovacik <mkovacik at redhat.com>
>>> wrote:
>>>
>>>> +0 who's the relevant party if not the commit bit owner?
>>>> +1 for commit bit owners receiving automatic notification to review
>>>>
>>>> --
>>>> milan
>>>>
>>>> On Tue, Aug 14, 2018 at 12:56 AM, Daniel Alley <dalley at redhat.com>
>>>> wrote:
>>>>
>>>>> +1. My understanding is that this will not directly limit who can
>>>>> review or merge code, but should streamline the review process by notifying
>>>>> relevant parties?
>>>>>
>>>>> On Mon, Aug 13, 2018 at 5:29 PM, David Davis <daviddavis at redhat.com>
>>>>> wrote:
>>>>>
>>>>>> We have come up with initial proposal of how to use code owners
>>>>>> feature in Pulp. Feedback on the initial proposal below is welcome. I will
>>>>>> try to collect the feedback and open a PUP by the end of the week. Thanks!
>>>>>>
>>>>>>
>>>>>> # Problem Statement
>>>>>>
>>>>>> For Pulp's review process, there are several areas we could improve:
>>>>>>
>>>>>> 1. It’s not always clear who should review files. Over time we have
>>>>>> developed subject matter experts for different areas of the codebase, but
>>>>>> these are not codified anywhere. It would be useful for us to define teams
>>>>>> need to review projects using code owners.
>>>>>>
>>>>>> 2. PRs go unnoticed. Github has a request-review feature, but only
>>>>>> members of the github organization can click 'request review' button. It
>>>>>> would be great if when a PR is opened people automatically received PR
>>>>>> review requests.
>>>>>>
>>>>>>
>>>>>> # Team Examples
>>>>>>
>>>>>> Functional Tests - The QE Team should be code owners of functional
>>>>>> tests that test core or core-maintained plugins
>>>>>> The Tasking System  - bmbouter, daviddavis, and dalley are the SME in
>>>>>> this area
>>>>>>
>>>>>>
>>>>>> # Solution
>>>>>>
>>>>>> 1. Configure the code-owners feature of Github (
>>>>>> https://blog.github.com/2017-07-06-introducing-code-owners/). It
>>>>>> will allow a team of 2 or more people to be notified and asked for review
>>>>>> when a PR modifies a file within a certain area of the code.
>>>>>>
>>>>>> 2. Require code-owner review to merge. This is described in this
>>>>>> section: https://blog.github.com/2017-07-06-introducing-code-owners/
>>>>>> #an-extra-layer-of-code-security
>>>>>>
>>>>>>
>>>>>> # Process
>>>>>>
>>>>>> The code owner role is not related to the commit bit. It's designed
>>>>>> to get better reviews. Well reviewed work can be confidently merged by
>>>>>> anyone with the commit bit.
>>>>>>
>>>>>> To make a change to code owners, open a PR with the changes, and call
>>>>>> for a lazy consensus vote by mailing list. Similar to the PUP decision
>>>>>> making process, voting must be open for 10 days, and the committers of the
>>>>>> respective repository are voting.
>>>>>>
>>>>>> The code owners file itself should be owned by the core committers of
>>>>>> the repository.
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Pulp-dev mailing list
>>>>>> Pulp-dev at redhat.com
>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Pulp-dev mailing list
>>>>> Pulp-dev at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>
>>>>>
>>>> _______________________________________________
>>>> Pulp-dev mailing list
>>>> Pulp-dev at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180814/2ceac6bb/attachment.htm>


More information about the Pulp-dev mailing list