I know I said that code owners should be SMEs but I don’t think it should it should be a strict requirement. The term SME and what constitutes an SME are nebulous and hard to define. I think the lazy consensus model would be a good first start as determining who qualifies for code ownership.
For having a bot automatically merge PRs, I can think of some examples of where we wouldn't want to merge automatically. Maybe there’s a way to handle them though? I wonder if this ought to be a separate conversation. David On Tue, Aug 14, 2018 at 9:56 AM Milan Kovacik <mkova...@redhat.com> wrote: > > > On Tue, Aug 14, 2018 at 3:47 PM, David Davis <davidda...@redhat.com> > wrote: > >> On Tue, Aug 14, 2018 at 9:35 AM Milan Kovacik <mkova...@redhat.com> >> wrote: >> >>> >>> >>> On Tue, Aug 14, 2018 at 1:26 PM, David Davis <davidda...@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 <mkova...@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 <dal...@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 <davidda...@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@redhat.com >>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>>> >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> Pulp-dev mailing list >>>>>> Pulp-dev@redhat.com >>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> Pulp-dev mailing list >>>>> Pulp-dev@redhat.com >>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>> >>>> >>> >
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
