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 <amacd...@redhat.com> 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: > > https://help.github.com/articles/about-pull-request-reviews/ > > > > 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 > > 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
