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

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Sat, May 4, 2024 at 1:57 AM Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> On 5/3/2024 4:38 PM, Michael D Kinney wrote:
> >
> >
> >> -----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 didn't see this explicitly mentioned but the easiest way to get to the
> PR if you already have a commit hash/URL like
> https://github.com/tianocore/edk2/commit/032830e is to click the PR link
> next to the branch name at the bottom of the commit message.
>
> In that commit you'll see: "master (#5560)"
>
> Where "#5560" is the PR number and the link to the PR.

Thank you Mike(s), this is useful stuff (and should be written down)!

-- 
Pedro


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