[Pulp-dev] PR merging

Daniel Alley dalley at redhat.com
Tue Sep 29 18:03:12 UTC 2020


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 <daviddavis at 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 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/20200929/cd1c36fe/attachment.htm>


More information about the Pulp-dev mailing list