Some times a conflict resolution is required when merging forward, so we would need to make sure that non-fast-forward case is handled.
Michael On Tue, Oct 4, 2016 at 11:48 AM, Brian Bouterse <[email protected]> wrote: > 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 <[email protected] >> <mailto:[email protected]>> 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 >> > [email protected] <mailto:[email protected]> >> > https://www.redhat.com/mailman/listinfo/pulp-dev >> <https://www.redhat.com/mailman/listinfo/pulp-dev> >> >> >> >> _______________________________________________ >> Pulp-dev mailing list >> [email protected] <mailto:[email protected]> >> https://www.redhat.com/mailman/listinfo/pulp-dev >> <https://www.redhat.com/mailman/listinfo/pulp-dev> >> >> >> >> >> _______________________________________________ >> Pulp-dev mailing list >> [email protected] >> https://www.redhat.com/mailman/listinfo/pulp-dev >> >> > _______________________________________________ > Pulp-dev mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/pulp-dev >
_______________________________________________ Pulp-dev mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-dev
