[Pulp-dev] RFC: Replace LGTM with github reviews

[Michael Hrivnak]

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 <amacdona at 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 at redhat.com
> > https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20161004/d4223730/attachment.htm>