[Pulp-dev] Pulp Code Owners

Milan Kovacik mkovacik at redhat.com
Tue Aug 14 14:50:02 UTC 2018


On Tue, Aug 14, 2018 at 4:29 PM, Ina Panova <ipanova at redhat.com> wrote:

> +1 for the pup.
>
> Milan,
>
> I guess you are SME, when you are publicly recognized to understand the
> topic.
> You will ask *when* this public recognition is happening?  When the answer
> to the such question like:
> -Who is person to contact for X?
> And the answer will be :
> - It's this guy Y.
>

How does one become the guy Y?
Why isn't guy Y a commit bit owner, if he/she's essential for the
commit/non-commit decision process?

--
milan


>
>
> --------
> Regards,
>
> Ina Panova
> Software Engineer| Pulp| Red Hat Inc.
>
> "Do not go where the path may lead,
>  go instead where there is no path and leave a trail."
>
> On Tue, Aug 14, 2018 at 3:56 PM, Milan Kovacik <mkovacik at redhat.com>
> wrote:
>
>>
>>
>> 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-0
>>>>>>>> 7-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
>>>>>>
>>>>>
>>>>
>>
>> _______________________________________________
>> 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/95c7dec9/attachment.htm>


More information about the Pulp-dev mailing list