Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process

["Michael D Kinney" <michael.d.kinney@intel.com>]

Rebecca,

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.

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Sunday, May 10, 2020 2:44 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; rfc@edk2.groups.io
> Subject: Re: [edk2-devel] [edk2-rfc] GitHub Pull
> Request based Code Review Process
> 
> Mike,
> 
> On 5/10/20 3:29 PM, Michael D Kinney wrote:
> 
> > There is no difference between CI checks run during
> code review
> > and the final CI checks before merge.  I think it is
> an interesting
> > conversation to decide how many times those CI checks
> should be
> > run and if they should run automatically on every
> change 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
> gives the
> _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 the
> git history
> > is linear with not git merges and supports both
> single patches
> > and patch series without squashing.  GitHub merge
> button 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
> important.
> 
> https://help.github.com/en/github/administering-a-
> repository/configuring-commit-squashing-for-pull-
> requests
> 
> 
> --
> Rebecca Cran
>