From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 03 Sep 2019 09:55:20 -0700 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 547C5796EE; Tue, 3 Sep 2019 16:55:20 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-140.ams2.redhat.com [10.36.116.140]) by smtp.corp.redhat.com (Postfix) with ESMTP id DAC6A5C21A; Tue, 3 Sep 2019 16:55:18 +0000 (UTC) Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1 To: "Ni, Ray" , "devel@edk2.groups.io" , "rfc@edk2.groups.io" , "Gao, Liming" , "Kinney, Michael D" References: <4A89E2EF3DFEDB4C8BFDE51014F606A14E4E1317@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C2BD07D@SHSMSX104.ccr.corp.intel.com> <8b35c38b-f914-42b6-e589-871ec92da713@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C2BE925@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 3 Sep 2019 18:55:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C2BE925@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 03 Sep 2019 16:55:20 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/03/19 18:41, Ni, Ray wrote: >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Laszlo Ersek >> Sent: Tuesday, September 3, 2019 6:20 AM >> To: Ni, Ray ; rfc@edk2.groups.io; devel@edk2.groups.io; Gao, Liming ; Kinney, >> Michael D >> 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