<div dir="ltr">Some times a conflict resolution is required when merging forward, so we would need to make sure that non-fast-forward case is handled.<div> </div><div>Michael</div></div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Oct 4, 2016 at 11:48 AM, Brian Bouterse <<a href="mailto:bbouters@redhat.com" target="_blank">bbouters@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It would still allow for merge forwards I think because: - The merge forward would happen locally (github can't stop that) - The pushes would be fast-forward which our protected branch policy allows. That's what I expect at least. -Brian On 10/04/2016 11:45 AM, Michael Hrivnak wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Big +1 to using the review feature. 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. 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? Michael On Tue, Oct 4, 2016 at 11:06 AM, Austin Macdonald <<a href="mailto:amacdona@redhat.com" target="_blank">amacdona@redhat.com</a> <mailto:<a href="mailto:amacdona@redhat.com" target="_blank">amacdona@redhat.com</a>>> wrote: On 10/03/2016 05:43 PM, Sean Myers wrote: > Github's new review feature is supported by another new github feature, > protected branches. If wanted to, we could combine the two ideas, and > only push to protected branches if there is at least one approved review > and no review with required changes outstanding. > > I think this could be used instead of the "LGTM" label I don't disagree, but I do like being able to see whether a PR has been approved or not at a glance in the PR list. > , and can also help > with the notion of "drive-by" reviews, since the UI gives you a clear > distinction between comments that require changes to be made and comments > that do not require changes to be made. > > For reference: > <a href="https://help.github.com/articles/about-pull-request-reviews/" rel="noreferrer" target="_blank">https://help.github.com/articles/about-pull-request-reviews/</a> <<a href="https://help.github.com/articles/about-pull-request-reviews/" rel="noreferrer" target="_blank">https://help.github.com/articles/about-pull-request-reviews/</a>> > > Since we've already got support for protected branches, I propose we > enable required reviews, and stop using the LGTM label. > > > > _______________________________________________ > Pulp-dev mailing list > <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <mailto:<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a>> > <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> <<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a>> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <mailto:<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a>> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> <<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a>> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> </blockquote><div class="HOEnZb"><div class="h5"> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> </div></div></blockquote></div> </div>