> -----Original Message----- > From: r...@edk2.groups.io <r...@edk2.groups.io> On Behalf Of Pedro Falcato > Sent: Thursday, May 2, 2024 10:51 AM > To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kin...@intel.com> > Cc: r...@edk2.groups.io; Leif Lindholm <l...@nuviainc.com>; Andrew Fish > (af...@apple.com) <af...@apple.com> > Subject: Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code > Review from email to GitHub Pull Requests on 5-24-2024 > > On Wed, May 1, 2024 at 6:44 PM Michael D Kinney via groups.io > <michael.d.kinney=intel....@groups.io> wrote: > > > > Hello, > > > > I would like to propose that TianoCore move all code review from email > > based code reviews to GitHub Pull Requests based code reviews. > > > > The proposed date to switch would be immediately after the next stable > > tag which is currently scheduled for May 24, 2024. > > > > 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. > > > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II- > Development-Process > > > > A couple examples of the changes that would need to be documented are: > > > > * 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. > > I'd just like to note that losing the CC:, Reviewed-by:, etc is a big > loss. Gerrit auto-adds Rb's, github PR's do not (I'd guess there's a > way to pull that off with github actions, but I haven't looked). It'll > be a mess if I have to go through online GH PR backlogs just to find > who to CC/add-to-review. It kills the decentralized bit off of git too > :) >
Can you provide more details on the impact of the loss? I am curious how other GitHub projects handle this topic. I see it is possible with squash and merge to amend the commit message, but I do not see any features that allow all the commits in a series to be amended. TianoCore does not use squash and merge. The main reason to remove this edit of the commit messages is that the commit message edits look like a new version of the series to git/GitHub and requires review again. This extra edit is also a manual process and there is a history of maintainers missing who did the reviews and acks. The state in the GitHub PR is always accurate. > > * 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' > label. > > > > > > Please provide feedback > > 1) If you are not in favor of this change. > > It is sad that we're moving to PRs after I finally got a nice and > sane(ish!) email workflow (openfw.io + b4). Otherwise, no objections, > it's better than edk2.git's half-email half-PR frankenprocess. > I'd guess this change only encompasses edk2.git? How about the other > repos? Any timeline for those? The plan is to apply this to all repos, one at a time. Need to get the revised process documented and working in one repo before applying to all. > > -- > Pedro > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118539): https://edk2.groups.io/g/devel/message/118539 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
