<div dir="ltr">Big +1 to using the review feature.<div><br></div><div>Although like Austin, I really like seeing in the PR list which have been LGTM'd. Maybe it's worth not having that anymore as the price for using the review feature, but it would be a shame to lose.</div><div><br></div><div>On mixing this with the protected branch feature, that sounds good. Would that still allow merging forward? For example, if I have a PR to 2.10-dev that's been reviewed and approved, once I merge it to 2.10-dev, can I merge that change to master and push the master branch?</div><div><br></div><div>Michael</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 4, 2016 at 11:06 AM, Austin Macdonald <span dir="ltr"><<a href="mailto:amacdona@redhat.com" target="_blank">amacdona@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 10/03/2016 05:43 PM, Sean Myers wrote:<br>
</span><span class="">> Github's new review feature is supported by another new github feature,<br>
> protected branches. If wanted to, we could combine the two ideas, and<br>
> only push to protected branches if there is at least one approved review<br>
> and no review with required changes outstanding.<br>
><br>
> I think this could be used instead of the "LGTM" label<br>
</span>I don't disagree, but I do like being able to see whether a PR has been<br>
approved or not at a glance in the PR list.<br>
<span class="im HOEnZb">> , and can also help<br>
> with the notion of "drive-by" reviews, since the UI gives you a clear<br>
> distinction between comments that require changes to be made and comments<br>
> that do not require changes to be made.<br>
><br>
> For reference:<br>
> <a href="https://help.github.com/articles/about-pull-request-reviews/" rel="noreferrer" target="_blank">https://help.github.com/<wbr>articles/about-pull-request-<wbr>reviews/</a><br>
><br>
> Since we've already got support for protected branches, I propose we<br>
> enable required reviews, and stop using the LGTM label.<br>
><br>
><br>
><br>
</span><div class="HOEnZb"><div class="h5">> ______________________________<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>
<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>