Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024

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

> -----Original Message-----
> From: rfc@edk2.groups.io <rfc@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.kinney@intel.com>
> Cc: rfc@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Andrew Fish
> (afish@apple.com) <afish@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.com@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-