public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Brian J. Johnson" <brian.johnson@hpe.com>
To: <rfc@edk2.groups.io>, <pedro.falcato@gmail.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "devel@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
Date: Fri, 10 May 2024 15:57:58 -0500	[thread overview]
Message-ID: <9b3986ab-770a-49dd-b68f-390623de9a64@hpe.com> (raw)
In-Reply-To: <CAKbZUD3WGMQmMUmdO1mU3kKmHRhhcjCLATGhqbWSa9ehNaiz8Q@mail.gmail.com>

On 5/3/24 12:38, Pedro Falcato wrote:
> 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).
> 

Well said.  That's my concern as well:  TianoCore won't use GitHub 
forever, and any GitHub metadata (PR numbers, GitHub IDs, bug numbers, 
etc.) will become meaningless once we change.  Never mind that the code 
can be disassociated from the metadata simply by forking to a new 
repository, as Pedro said....

> 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?
> 
>>
>> 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 :)

Yeah... my opinions are decidedly mixed.  They are convenient, but have 
some serious gaps around archiving, auditing, and versioning of review 
requests.  They don't even let you review the commit messages (one of 
their most serious flaws!)

> <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!
> 

-- 
Brian J. Johnson
Hewlett Packard Enterprise


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



  parent reply	other threads:[~2024-05-10 20:58 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-01 17:43 [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024 Michael D Kinney
2024-05-01 18:12 ` Leif Lindholm
2024-05-01 23:19   ` Dionna Glaze via groups.io
2024-05-02 15:59     ` Brian J. Johnson
2024-05-02 16:09       ` Dionna Glaze via groups.io
2024-05-02 16:30       ` [edk2-rfc] " Michael D Kinney
2024-05-06 16:41   ` Leara, William via groups.io
2024-05-02  1:28 ` Rebecca Cran
2024-05-02 10:21   ` Leif Lindholm
2024-05-02  3:08 ` Michael Kubacki
2024-05-02 10:57   ` Leif Lindholm
2024-05-02 16:37     ` Michael D Kinney
2024-05-03 16:58       ` Michael Kubacki
2024-05-03 16:54     ` Michael Kubacki
2024-05-02  6:33 ` Marcin Juszkiewicz
2024-05-02 10:34   ` Leif Lindholm
2024-05-02 15:21     ` Michael Kubacki
2024-05-02 16:24       ` Michael D Kinney
2024-05-03 17:21         ` Michael Kubacki
2024-05-03 19:16           ` [edk2-rfc] " Michael D Kinney
2024-05-02  9:37 ` Ard Biesheuvel
2024-05-02 15:14   ` Michael Kubacki
2024-05-03  0:35     ` [edk2-rfc] " Rebecca Cran
2024-05-02 17:50 ` Pedro Falcato
2024-05-02 18:17   ` [edk2-rfc] " Michael D Kinney
2024-05-03 17:38     ` Pedro Falcato
2024-05-03 20:12       ` Michael D Kinney
2024-05-03 20:38         ` Michael D Kinney
2024-05-04  0:57           ` Michael Kubacki
2024-05-05 18:10             ` Pedro Falcato
2024-05-06 10:00           ` Ard Biesheuvel
2024-05-06 15:11             ` Michael D Kinney
2024-05-06 15:30               ` Ard Biesheuvel
2024-05-06 15:56                 ` Michael D Kinney
2024-05-06 16:09                   ` Ard Biesheuvel
2024-05-10 20:57       ` Brian J. Johnson [this message]
2024-05-15 17:03         ` Michael D Kinney
2024-05-24 12:20 ` [edk2-devel] [edk2-rfc] " Rebecca Cran
2024-05-24 14:53   ` Michael Kubacki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9b3986ab-770a-49dd-b68f-390623de9a64@hpe.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox