<div dir="ltr"><div>+1 for the pup.</div><div><br></div><div>Milan,</div><div><br></div><div>I guess you are SME, when you are publicly recognized to understand the topic.</div><div>You will ask *when* this public recognition is happening?  When the answer to the such question like:</div><div>-Who is person to contact for X?</div><div>And the answer will be :</div><div>- It's this guy Y.<br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><br><br>--------<br>Regards,<br><br>Ina Panova<br>Software Engineer| Pulp| Red Hat Inc.<br><br>"Do not go where the path may lead,<br> go instead where there is no path and leave a trail."<br></div></div></div>
<br><div class="gmail_quote">On Tue, Aug 14, 2018 at 3:56 PM, Milan Kovacik <span dir="ltr"><<a href="mailto:mkovacik@redhat.com" target="_blank">mkovacik@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Aug 14, 2018 at 3:47 PM, David Davis <span dir="ltr"><<a href="mailto:daviddavis@redhat.com" target="_blank">daviddavis@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span>On Tue, Aug 14, 2018 at 9:35 AM Milan Kovacik <<a href="mailto:mkovacik@redhat.com" target="_blank">mkovacik@redhat.com</a>> wrote:<br></span><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 14, 2018 at 1:26 PM, David Davis <span dir="ltr"><<a href="mailto:daviddavis@redhat.com" target="_blank">daviddavis@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.</div></blockquote><div><br></div><div> So what you mean is actually a trusted subset of commit bit owners, like the SMEs?</div></div></div></div></blockquote><div><br></div></span><div>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.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> So we don't trust all commit bit owners equally when it comes to particular git (sub)tree?</div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> 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?</div><div> Or is it rather about decoupling the code review duty from the commit bit ownership?<br></div></div></div></div></blockquote><div><br></div></span><div>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.</div></div></div></blockquote><div><br></div></span><div>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?<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div> Why do we need commit bit owners then?</div></div></div></div></blockquote><div><br></div></span><div>How else do we merge the code if no one has a commit bit?</div></div></div></blockquote><div><br></div></span><div>  thru a bot for instance<br></div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><div class="m_-7155717772419181885h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Cheers,</div><div>milan<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>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.</div><span class="m_-7155717772419181885m_-6585431531302923894m_-4553150148462957757m_8781948417688011146m_-8369782630327176947m_-6637270726129223690HOEnZb"><font color="#888888"><div><br><div><div dir="ltr" class="m_-7155717772419181885m_-6585431531302923894m_-4553150148462957757m_8781948417688011146m_-8369782630327176947m_-6637270726129223690m_-3746685829730664133gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>David<br></div></div></div></div></div></div></div></div><br></div></font></span></div><div class="m_-7155717772419181885m_-6585431531302923894m_-4553150148462957757m_8781948417688011146m_-8369782630327176947m_-6637270726129223690HOEnZb"><div class="m_-7155717772419181885m_-6585431531302923894m_-4553150148462957757m_8781948417688011146m_-8369782630327176947m_-6637270726129223690h5"><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 14, 2018 at 6:35 AM Milan Kovacik <<a href="mailto:mkovacik@redhat.com" target="_blank">mkovacik@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>+0 who's the relevant party if not the commit bit owner?</div><div>+1 for commit bit owners receiving automatic notification to review</div><div><br></div><div>--</div><div>milan<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 14, 2018 at 12:56 AM, Daniel Alley <span dir="ltr"><<a href="mailto:dalley@redhat.com" target="_blank">dalley@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">+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?<br><div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-7155717772419181885m_-6585431531302923894m_-4553150148462957757m_8781948417688011146m_-8369782630327176947m_-6637270726129223690m_-3746685829730664133m_4949605523463309318h5">On Mon, Aug 13, 2018 at 5:29 PM, David Davis <span dir="ltr"><<a href="mailto:daviddavis@redhat.com" target="_blank">daviddavis@redhat.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_-7155717772419181885m_-6585431531302923894m_-4553150148462957757m_8781948417688011146m_-8369782630327176947m_-6637270726129223690m_-3746685829730664133m_4949605523463309318h5"><div dir="ltr"><div>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!</div><div><br></div><div><br></div><div># Problem Statement</div><div><br></div><div>For Pulp's review process, there are several areas we could improve:</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div><br></div><div># Team Examples</div><div><br></div><div>Functional Tests - The QE Team should be code owners of functional tests that test core or core-maintained plugins</div><div>The Tasking System  - bmbouter, daviddavis, and dalley are the SME in this area</div><div><br></div><div><br></div><div># Solution</div><div><br></div><div>1. Configure the code-owners feature of Github (<a href="https://blog.github.com/2017-07-06-introducing-code-owners/" target="_blank">https://blog.github.com/2017-<wbr>07-06-introducing-code-owners/</a><wbr>). 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.</div><div><br></div><div>2. Require code-owner review to merge. This is described in this section: <a href="https://blog.github.com/2017-07-06-introducing-code-owners/#an-extra-layer-of-code-security" target="_blank">https://blog.github.com/2017-0<wbr>7-06-introducing-code-owners/#<wbr>an-extra-layer-of-code-securit<wbr>y</a></div><div><br></div><div><br></div><div># Process</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>The code owners file itself should be owned by the core committers of the repository.</div><div><br></div></div>
<br></div></div>______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman<wbr>/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div></div></div>
<br>______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman<wbr>/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div>
______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman<wbr>/listinfo/pulp-dev</a><br>
</blockquote></div>
</div></div></blockquote></div><br></div></div>
</blockquote></div></div></div></div>
</blockquote></div></div></div><br></div></div>
<br>______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div>