<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 14, 2018 at 5:11 PM, Dana Walker <span dir="ltr"><<a href="mailto:dawalker@redhat.com" target="_blank">dawalker@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"><div>Can this idea of SME as code owner be used *only* for notification so that we don't get flooded with them for every PR across the codebase and *not* as a requirement for merging so other still can get things rolling, this just covers the scenarios where a PR fell between the cracks unnoticed?</div></div></blockquote><div><br></div><div>lazy myself excuse would be: meh I'm not an SME here, I don't review that PR part</div><div><br></div><div>My concern is we should use the tools in a way that motivate reviews to both spread the domain knowledge (i.e to become a commit bit owner and/or an SME eventually) and help the review process speed and quality.</div><div><br></div><div>I suggest adopting formal criteria that would promote this; implementation can be thru Code owners just mind to create/update the codeowners file a commit bit is required too.<br></div><div><br></div><div>--</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>--Dana<br></div></div><div class="gmail_extra"><br clear="all"><div><div class="m_-2332625127146650398gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div>
<p style="font-weight:bold;margin:0;padding:0;font-size:14px;text-transform:uppercase;margin-bottom:0"><span>Dana</span> <span>Walker</span></p>
<p style="font-weight:normal;font-size:10px;margin:0px 0px 4px;text-transform:uppercase"><span>Associate Software Engineer</span><span style="font-weight:normal;color:#aaa;margin:0"></span></p>
<p style="font-weight:normal;margin:0;font-size:10px;color:#999"><a style="color:#0088ce;font-size:10px;margin:0;text-decoration:none;font-family:'overpass',sans-serif" href="https://www.redhat.com" target="_blank">Red Hat <span><br><br></span></a></p>




<table border="0"><tbody><tr><td width="100px"><a href="https://red.ht/sig" target="_blank"> <img src="https://www.redhat.com/files/brand/email/sig-redhat.png" width="90" height="auto"></a> </td>
</tr></tbody></table>

</div></div></div></div>
<br><div class="gmail_quote"><div><div class="h5">On Tue, Aug 14, 2018 at 11:05 AM, 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="h5"><div dir="ltr">The code owner feature in Github requests reviews from code owners when a PR is opened. This both notifies the responsible team when they have a PR to review and also makes it clear who should be reviewing a particular PR so that the PR author can follow up with those people.<span class="m_-2332625127146650398HOEnZb"><font color="#888888"><div><div><div><div><div dir="ltr" class="m_-2332625127146650398m_-8365240814814852331m_-4544786066920661112m_5940815626755237823gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><br></div><div>David<br></div></div></div></div></div></div></div></div><br></div></div></div></font></span></div><div class="m_-2332625127146650398HOEnZb"><div class="m_-2332625127146650398h5"><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 14, 2018 at 10:36 AM Bryan Kearney <<a href="mailto:bkearney@redhat.com" target="_blank">bkearney@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 08/13/2018 05:29 PM, David Davis wrote:<br>
> <br>
> <br>
> # Problem Statement<br>
> <br>
> For Pulp's review process, there are several areas we could improve:<br>
> <br>
> 1. It’s not always clear who should review files. Over time we have<br>
> developed subject matter experts for different areas of the codebase,<br>
> but these are not codified anywhere. It would be useful for us to define<br>
> teams need to review projects using code owners.<br>
> <br>
> 2. PRs go unnoticed. Github has a request-review feature, but only<br>
> members of the github organization can click 'request review' button. It<br>
> would be great if when a PR is opened people automatically received PR<br>
> review requests.<br>
> <br>
> <br>
> # Team Examples<br>
> <br>
> Functional Tests - The QE Team should be code owners of functional tests<br>
> that test core or core-maintained plugins<br>
> The Tasking System  - bmbouter, daviddavis, and dalley are the SME in<br>
> this area<br>
> <br>
> <br>
> # Solution<br>
> <br>
> 1. Configure the code-owners feature of Github<br>
> (<a href="https://blog.github.com/2017-07-06-introducing-code-owners/" rel="noreferrer" target="_blank">https://blog.github.com/2017-<wbr>07-06-introducing-code-owners/</a><wbr>). It will<br>
> allow a team of 2 or more people to be notified and asked for review<br>
> when a PR modifies a file within a certain area of the code.<br>
> <br>
> 2. Require code-owner review to merge. This is described in this<br>
> section:<br>
> <a href="https://blog.github.com/2017-07-06-introducing-code-owners/#an-extra-layer-of-code-security" rel="noreferrer" target="_blank">https://blog.github.com/2017-0<wbr>7-06-introducing-code-owners/#<wbr>an-extra-layer-of-code-securit<wbr>y</a><br>
> <br>
> <br>
> # Process<br>
> <br>
> The code owner role is not related to the commit bit. It's designed to<br>
> get better reviews. Well reviewed work can be confidently merged by<br>
> anyone with the commit bit.<br>
> <br>
> To make a change to code owners, open a PR with the changes, and call<br>
> for a lazy consensus vote by mailing list. Similar to the PUP decision<br>
> making process, voting must be open for 10 days, and the committers of<br>
> the respective repository are voting.<br>
> <br>
> The code owners file itself should be owned by the core committers of<br>
> the repository.<br>
> <br>
If the problem statement is a slowdown in PRs, how does limiting who can<br>
do the review/merge solve the issue?<br>
<br>
-- bk<br>
<br>
<br>
</blockquote></div>
</div></div><br></div></div><span class="">______________________________<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></span></blockquote></div><br></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></div>