From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 58F32AC09B1 for ; Thu, 2 May 2024 03:08:39 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=DBeDyXJPsBthxyVW27h57J/SsPlih3c9bnQmuPdVVe8=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240206; t=1714619318; v=1; b=UwhB2PParFdn2ETIawKxF5VaMJX8zuKIgKHERXa0EVcixmEn8//47viwU8hGGkUce/GRKsFH szOdZci7qTMRimj9ZFQewJpBICID+L6XUOlcTHmYdc2CeZTQ4Dzyqgy5yv0fgFddDQzFMLkLDdi kwN+r59cS8TOlPhtkVtCJXHeqH8PFfrC29kqud8qekGw3MTIe6IhXk8zZ8wvM+z6WZ5oTAORITb Hmx98gAhTLeJyYSo6OrzIVaqGlEHNJwLyjLQBZJzEstIBSp0GuxlzH62i0lQwSP5v3uFtE8fupY UtF8JAL+VsCPtig37LvEiMFy6DjyvEjl8WIYiPCsEUsBA== X-Received: by 127.0.0.2 with SMTP id NdlNYY7687511xdar8x6IE6K; Wed, 01 May 2024 20:08:38 -0700 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.4864.1714619317185902874 for ; Wed, 01 May 2024 20:08:37 -0700 X-Received: from [10.6.0.181] (unknown [20.39.63.8]) by linux.microsoft.com (Postfix) with ESMTPSA id 24B4B206B4DD; Wed, 1 May 2024 20:08:36 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 24B4B206B4DD Message-ID: <5116382f-a1cf-46fc-95a2-58c7f810765d@linux.microsoft.com> Date: Wed, 1 May 2024 20:08:34 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024 To: devel@edk2.groups.io, michael.d.kinney@intel.com, "rfc@edk2.groups.io" Cc: Leif Lindholm , "Andrew Fish (afish@apple.com)" References: From: "Michael Kubacki" In-Reply-To: Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Wed, 01 May 2024 20:08:37 -0700 Resent-From: mikuback@linux.microsoft.com Reply-To: devel@edk2.groups.io,mikuback@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: qYI17BPrqDyfjI9gjOhXQdFTx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=UwhB2PPa; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io Thank you for this proposal. We've been anticipating this change for=20 years and are excited to help support it. Here's some items we'd like to raise for feedback that we could help=20 implement. Many could likely be done in time for the transition. 1. Automate reviewers - We've discussed CODEOWNERS in the past. However,=20 a simpler approach (in maintaining/syncing less files) would be to use=20 Maintainers.txt directly with a GitHub workflow since the file already=20 contains GitHub IDs. 2. Make PR completion contingent on a GitHub review from at least one=20 package maintainer/reviewer for each package in the PR. 3. Dependabot is already used today to automatically create PRs when=20 dependencies like pip modules have updates. To allow this to more=20 effectively keep dependencies up-to-date, allow dependabot PRs to be=20 completed (after normal acceptance criteria like CI and review=20 requirements) without a separate human creating a duplicate PR. 4. Potentially warn users (with an automated comment on the PR) if they=20 add a push label to a PR that is less than 24 hours old. 5. Leave reminder comments on PRs with absolutely no activity after some=20 agreed upon time so reviewers are notified to review the PR without the=20 submitter having to watch it and send notifications. 6. Leave reminder comments on PRs that meet all requirements to be=20 completed (all reviews accounted for and status checks pass) but are=20 still open so those on the PR are notified to complete it without the=20 submitter having to manually watch and send reminders. 7. We are happy to help with process documentation. These are items we think can help facilitate consistency and efficiency=20 of contributions. --- Question: Are you planning to reset review state upon force pushes to=20 the PR or allow prior reviews to apply? --- Notes: - We've found a PR template helps produce higher quality and consistent=20 PR messages. That could be added if there's interest. - We've also found an automated PR description review tool helps produce=20 higher quality PR descriptions which allow reviewers to understand the=20 PR more quickly and allow any release note automation to produce higher=20 quality release notes. - We might want to consider utilizing labels better to categorize PR=20 impact. For example, "bug", "breaking-change", "new-feature", etc. These=20 help with PR searches and PR data queries. - We've automated release note generation which can categorize changes=20 by impact (using labels). This could be useful to produce more detailed=20 and informational GitHub release notes for stable tags. - As you likely know, conversation resolution is a simple option to enforce= . Thanks, Michael On 5/1/2024 1:43 PM, Michael D Kinney wrote: > Hello, >=20 > I would like to propose that TianoCore move all code review from email > based code reviews to GitHub Pull Requests based code reviews. >=20 > The proposed date to switch would be immediately after the next stable > tag which is currently scheduled for May 24, 2024. >=20 > Updates to the following Wiki page would be required to describe the > required process when using GitHub Pull Requests for all code review > related activity. >=20 > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Develop= ment-Process >=20 > A couple examples of the changes that would need to be documented are: >=20 > * All contributors, maintainers, and reviewers must have GitHub IDs. > * The commit message would no longer require Cc:, Reviewed-by:, Acked-by: > or Tested-by: tags. The only required tag would be Signed-off-by. > * The Pull Request submitter is required to invite the required > maintainers and reviewers to the pull request. This is the same > set of maintainers and reviewers that are required to be listed in > Cc: tags in today's process. > * Maintainers are responsible for verifying that all conversations in > the code review are resolved and that all review approvals from the > required set of maintainers are present before setting the 'push' labe= l. >=20 >=20 > Please provide feedback > 1) If you are not in favor of this change. > 2) If you are not in favor of the proposed date of this change. > 3) On the process changes you would like to see documented in the Wiki > pages related to using GitHub Pull Request based code reviews. >=20 > There is some prototype work to automate/simplify some of the PR based > code review process steps. Those could be added over time as resources > are available to finish and support them. >=20 > Best regards, >=20 > Mike >=20 >=20 >=20 >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118491): https://edk2.groups.io/g/devel/message/118491 Mute This Topic: https://groups.io/mt/105847510/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-