Re: [edk2-devel] [RFC] EDK II Continuous Integration Phase 1


Laszlo Ersek
 

On 09/03/19 18:41, Ni, Ray wrote:
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, September 3, 2019 6:20 AM
To: Ni, Ray <ray.ni@intel.com>; rfc@edk2.groups.io; devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Kinney,
Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1

On 09/03/19 05:39, Ni, Ray wrote:
Can someone draw a flow chart of who does what to help clarify?
I don't see this as a huge change, relative to the current process.

Before, it's always been up to the subsys maintainer to apply & rebase
the patches locally, pick up the feedback tags, and run at least *some*
build tests before pushing.

After, the final git push is not to "origin" but to a personal branch on
github.com, and then a github PR is needed. If that fails, notify the
submitter. That's all, as far as I can see.

It sounds like maintainers are going to be the very important bridges between CI system and the patch owners (developers).
Important it is I agree but boring it is as well I have to say.

Oh, it *is* absolutely boring.

Are maintainers still needed to be picked up/chosen/promoted from developers?
Would you trust a person to apply, build-test, and push your UefiCpuPkg
patches, if that person had *zero* UefiCpuPkg development experience?
(Or even zero edk2 development experience?)
Can we change the process a bit?
1. maintainers created pull requests on behave of the patch owners
2. patch owners can be notified automatically if pull requests fail
I believe this can work if:
- the submitter has a github account
- the maintainer knows the submitter's account name
- the maintainer manually subscribes the submitter to the PR
(I have never tried this myself)

3. patch owners update the pull requests
(I am not familiar to pull requests. I assume the rights of modifying pull requests and creating pull requests are separated. Are they?)
PRs should not be updated. If there is a CI failure, the PR should be
rejected. If that happens automatically, that's great. If not, then
someone must close the PR manually. If the patch series submitter can do
that, that's great (I have no idea if that works!) If only the PR owner
(= maintainer) can close the PR, then they should close it.

Either way, the patch author will have to submit a v2 to the list. When
that is sufficiently reviewed, the same process starts (new PR opened by
maintainer).

So, maintainers only need to initiate the pull requests.
I hope this can actually work. We need to test this out first.

It assumes when pull requests are initiated, everyone at least agrees the justifications of the changes are valid and the ways the changes are done.
I agree fully about this. If the CI check completes, then the changes
are pushed to master automatically! (Only if the push is a fast-forward
-- otherwise the PR is rejected again. GitHub will not be allowed to
merge or rebase withoout supervision.)

Thanks
Laszlo

Join rfc@edk2.groups.io to automatically receive all group messages.