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: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Friday, May 3, 2024 1:13 PM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: rfc@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm
> <leif@nuviainc.com>; Andrew Fish (afish@apple.com) <afish@apple.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code
> Review from email to GitHub Pull Requests on 5-24-2024
> 
> 
> 
> > -----Original Message-----
> > From: Pedro Falcato <pedro.falcato@gmail.com>
> > Sent: Friday, May 3, 2024 10:39 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: rfc@edk2.groups.io; devel@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 Thu, May 2, 2024 at 7:17 PM Kinney, Michael D
> > <michael.d.kinney@intel.com> wrote:
> > >
> > >
> > >
> > > > -----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:
> > <snip>
> > > > > * 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?
> >
> > In my view, commits should be fairly self-describing. What changes,
> > why, are obvious, but who looked at it, who reviewed it, who was cc'd
> > but didn't respond, who tested are also pretty important. Git is
> > supposed to be decentralized, let's not forget. If we ever migrate
> > from GH, if GH ever goes down, if the links ever go down, you'll never
> > be able to know who looked at it. If you're looking at an EDK2 commit
> > deep into an Intel-internal fork, you won't know what "PR #478" is
> > (heck, rebase-and-merge doesn't reference PRs either).
> >
> > Side-note: How are we supposed to find the PR for a given commit?
> > Searching doesn't seem to work well. For instance, I picked a random
> > non-trivial commit out of the current open PRs:
> > MdeModulePkg/Bus/Spi/SpiBus: Adding SpiBus Drivers.
> >
> https://github.com/tianocore/edk2/pulls?q=is%3Apr+is%3Aopen+MdeModulePkg
> > %2FBus%2FSpi%2FSpiBus%3A+Adding+SpiBus+Drivers
> > has no matches?
> 
> If you have the sha of the commit, you can search in GitHub
> 
> For example, I selected a commit at random from recent edk2 commit
> history:
> 
> 	https://github.com/tianocore/edk2/commit/032830e96841f2a752e364378c
> 3428ac5d2f59d1
> 
> Goto the "Pull Requests" tab for the repo and in the "Filters" search
> box enter
> 
> 	is:pr is:merged <sha>
> 
> In this example:
> 
> 	is:pr is:merged 032830e96841f2a752e364378c3428ac5d2f59d1
> 
> This returns a single hit on PR #5560
> 
> 	https://github.com/tianocore/edk2/pull/5560
> 
> There is also a 'gh' command line utility that can be used to write
> small scripts to collect this information

Here is the equivalent query and output using 'gh' CLI command:

    gh pr list --repo tianocore/edk2 --state merged --search 032830e96841f2a752e364378c3428ac5d2f59d1

    Showing 1 of 1 pull request in tianocore/edk2 that matches your search

    ID     TITLE     BRANCH            CREATED AT
    #5560  Loongcpu  niruiyu:loongcpu  about 17 days ago


> 
> >
> > >
> > > I am curious how other GitHub projects handle this topic. I see it
> >
> > I don't think they do, sadly. But I also don't know many people with a
> > positive opinion on GH PRs :)
> >
> > <snip>
> > > > 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.
> >
> > Gotcha, thanks!
> >
> > --
> > Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118564): https://edk2.groups.io/g/devel/message/118564
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]
-=-=-=-=-=-=-=-=-=-=-=-