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
