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

[Brian Bouterse]

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:
> 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
> <mailto: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/
>     <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 <mailto:Pulp-dev at redhat.com>
>     > https://www.redhat.com/mailman/listinfo/pulp-dev
>     <https://www.redhat.com/mailman/listinfo/pulp-dev>
>
>
>
>     _______________________________________________
>     Pulp-dev mailing list
>     Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>     https://www.redhat.com/mailman/listinfo/pulp-dev
>     <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
>