toggle quoted messageShow quoted text
I agree that the first version should rerun CI checks
on every time commits are added to a PR or there is a
forced push to the PR.
Perhaps we should use Draft Pull Requests as a way
to indicate the content is not ready for code review
or CI checks yet.https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests#draft-pull-requests
We also want emails added to the email archive when
the pull request is either abandoned or merged.
merify can add comments to a PR that are picked up
by the webhook.
I agree with reducing the number of services required.
There was feedback from Laszlo related to rebase for
pull requests using the current CI process. I will
do more investigations of GitHub features, webhook
features, and Mergify features to see if there is
simpler overall solution.
From: Rebecca Cran <email@example.com>
Sent: Sunday, May 10, 2020 2:44 PM
To: firstname.lastname@example.org; Kinney, Michael D
Subject: Re: [edk2-devel] [edk2-rfc] GitHub Pull
Request based Code Review Process
On 5/10/20 3:29 PM, Michael D Kinney wrote:
There is no difference between CI checks run duringcode review
and the final CI checks before merge. I think it isan interesting
conversation to decide how many times those CI checksshould be
run and if they should run automatically on everychange during
review or on demand.I'd suggest following what other Github projects do,
which I think is to
run the CI checks automatically on every change that's
made in a pull
request - I don't know if it might also be necessary to
run them during
the merge, if master has changed in the meantime. That
_submitter_ feedback about any changes they need to
make, instead of
having to wait until the maintainer tells them their
change has broken
something: it speeds up the development process.
Mergify is more flexible. We want to make sure thegit history
is linear with not git merges and supports bothsingle patches
and patch series without squashing. GitHub mergebutton by
default squashes all commits into a single commit.Wouldn't disabling all but "Allow rebase merging" do
the same thing
without the additional potential failure point? Though
it sounds like
we've resolved the problems with Mergify, so it's not