I wasn't able to get to this this week. Will send out an email next week with another date.
David On Tue, Oct 13, 2020 at 12:52 PM David Davis <davidda...@redhat.com> wrote: > During the pulpcore team meeting today there was a desire to move forward > with auto-merging pulpcore PRs. This would mean automatically merging of > pulpcore PRs that have all the necessary approvals (2), tests passing, no > changes requested, and are not draft PRs. > > Here is the pulp-ci PR again for anyone that wants to review it: > https://github.com/pulp/pulp-ci/pull/737 > > Assuming there are no objections, the tentative date for this to take > effect will be Oct 23. I'll send out an announcement closer to that date as > a reminder along with info about how plugins can enable this feature too. > > David > > > On Thu, Oct 1, 2020 at 10:12 AM Brian Bouterse <bmbou...@redhat.com> > wrote: > >> >> >> On Thu, Oct 1, 2020 at 7:35 AM Ina Panova <ipan...@redhat.com> wrote: >> >>> >>> >>> -------- >>> Regards, >>> >>> Ina Panova >>> Senior Software Engineer| Pulp| Red Hat Inc. >>> >>> "Do not go where the path may lead, >>> go instead where there is no path and leave a trail." >>> >>> >>> On Wed, Sep 30, 2020 at 2:38 PM David Davis <davidda...@redhat.com> >>> wrote: >>> >>>> I am also concerned about the lack of human involvement and the >>>> potential for things to be merged accidentally. I definitely could see that >>>> happening. >>>> >>>> I like the idea of having the PR processor only merge if a label (eg >>>> "Merge when Ready") is present. The question then is whether it should be >>>> applied automatically or not when a PR is opened. Maybe to start out with, >>>> it's not. >>>> >>>> On one hand, I like the idea of having this label as well and adding it >>> to the already mentioned conditions in your PR[0]. However, I would imagine >>> this would be a manual step: whether the reviewer or the author of the PR >>> would set this label. This will also cover the case Daniel >>> mentioned.."sometimes there's only minor changes requested and it doesn't >>> warrant a re-review. Currently what we do pretty often is just approve the >>> PR, leave a note to change the wording here and there, and the author can >>> just merge it whenever they're done with the minor changes." >>> >>> On the other hand I don't know what will be easier: pressing the merge >>> button or manually adding the label. Probably the only benefit would be - >>> you don't need to wait and hypnotize the CI so it gets green. >>> >>> Openstack upstream policies also uses auto-merge [0] scroll to the end. >>> >>> In my opinion, if we want to automate things this will require us to be >>> more diligent and responsible and leave +1 only when the PR is *really* >>> ready. Having the PR in a not a not draft state, +2 approvals and passing >>> CI I think is satisfactory enough to be merged. Adding another label feels >>> like killing the whole gist of automation. >>> >> I agree. If we're adding automation but also with manual steps we're not >> really realizing the benefits the automation is supposed to give us. >> >> I do think automation with two acks is a good idea. To me, our acks >> indicate something is ready to merge, and if two committers are saying that >> is the case, I believe the machines can take it from there. >> >>> >>> Another thing that can be done is clearing up all the approvals on each >>> push to the PR. But this will require people approving once again. >>> >>> [0] >>> https://github.com/pulp/pulp-ci/pull/737/files#diff-cbdf61d38083f599940c37eeb49cb0a9R116-R141 >>> [1] https://docs.openstack.org/zaqar/latest/contributor/first_patch.html >>> >>> One other thing I thought of is that the PR processor could also check >>>> PR dependencies (ie "Required PRs") and see if they are all mergeable >>>> before merging any PR. >>>> >>>> David >>>> >>>> >>>> On Wed, Sep 30, 2020 at 4:10 AM Matthias Dellweg <mdell...@redhat.com> >>>> wrote: >>>> >>>>> This just reminds me that Gitlab has a very nice "merge when CI passes >>>>> button" to decouple the merge question from the reviews. >>>>> The only way i could see this happen in Github is via setting a label >>>>> that instructs the PR processor to merge when (label is set) && (ci is >>>>> green) && (other conditions). >>>>> Does not sound too user friendly, but labels can only be set by people >>>>> with merge permissions anyway (so not a security concern). >>>>> >>>>> On Tue, Sep 29, 2020 at 8:04 PM Daniel Alley <dal...@redhat.com> >>>>> wrote: >>>>> >>>>>> I have some doubts about the cost/benefit ratio of coding automation >>>>>> to merge PRs vs. simply changing the default option / being selective >>>>>> about >>>>>> which options are available. >>>>>> >>>>>> I like the idea in general though. A lot of projects do something >>>>>> like this. I occasionally contributed to the Servo project (RIP) and >>>>>> they >>>>>> used automation to manage this. The benefits were, their tests suites >>>>>> ran >>>>>> for nearly 2 hours on 3 different operating systems, so the feedback loop >>>>>> was quite slow, and automatically triggering the full test run every >>>>>> time a >>>>>> PR was changed would be a horrendous waste of resources at the scale they >>>>>> were running at. So basically they would run a minimal CI within the PR, >>>>>> then require a manual review, and once approved it would trigger the full >>>>>> CI, and if that passed then it would automatically merge the PR hours >>>>>> later >>>>>> whenever it finished. >>>>>> >>>>>> If our test suites keep getting longer and longer and take more than >>>>>> the 20-25 minutes they currently take, I can definitely see how the >>>>>> commit >>>>>> processor approach could add value. >>>>>> >>>>>> On the other hand, it has the downside that, sometimes even once a >>>>>> commit is approved you might want to change something minor like rebasing >>>>>> the commits manually, and automation might start getting in the way of >>>>>> things like that. Or, another example, sometimes there's only minor >>>>>> changes requested and it doesn't warrant a re-review. Currently what we >>>>>> do >>>>>> pretty often is just approve the PR, leave a note to change the wording >>>>>> here and there, and the author can just merge it whenever they're done >>>>>> with >>>>>> the minor changes. >>>>>> >>>>>> On Tue, Sep 29, 2020 at 12:49 PM David Davis <davidda...@redhat.com> >>>>>> wrote: >>>>>> >>>>>>> Hi all, >>>>>>> >>>>>>> Last week the discussion about how to merge PRs got me >>>>>>> thinking about how we could potentially programmatically merge PRs. The >>>>>>> openshift project on github does this[0] and I wondered if it would work >>>>>>> for Pulp. >>>>>>> >>>>>>> I think the main benefits would be (1) we'd be able to code how to >>>>>>> best merge PRs and (2) we'd no longer see PRs sitting around that are >>>>>>> ready >>>>>>> to merge but not merged (granted this doesn't happen often). >>>>>>> >>>>>>> I created a PoC against the PR processor that's quite simple (here >>>>>>> are the relevant bits[1]). It's worth noting that we can still prevent >>>>>>> PRs >>>>>>> from being merged by setting them to drafts (I believe both the PR >>>>>>> author >>>>>>> or any repo owner do this) and the PR processor won't merge anything >>>>>>> unless >>>>>>> it's ready to be merged. >>>>>>> >>>>>>> I myself am a bit conflicted about this so feedback would be >>>>>>> appreciated. >>>>>>> >>>>>>> [0] https://github.com/openshift >>>>>>> [1] >>>>>>> https://github.com/pulp/pulp-ci/pull/737/files#diff-cbdf61d38083f599940c37eeb49cb0a9R116-R141 >>>>>>> >>>>>>> David >>>>>>> _______________________________________________ >>>>>>> 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 >>>> >>> _______________________________________________ >>> 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