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

["Michael Kubacki" <mikuback@linux.microsoft.com>]

On 5/2/2024 5:37 AM, Ard Biesheuvel wrote:
> On Wed, 1 May 2024 at 19:44, Michael D Kinney
> <michael.d.kinney@intel.com> 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.
>>
> 
> +1
> 
> Some kind of off-github reflector similar to rebecca's public inbox
> [0] would be appreciated to at least be able to keep track of
> different versions of a series exactly as they were submitted. One of
> the things that annoys me about doing GitHub reviews is dealing with
> PRs where the underlying branches gets updated through a force-push
> and there is no way to find out what changed, and whether the existing
> review comments are still in sync.
> 
> Would it be possible to require a separate PR for each revision of a
> series, lock the underlying branch, and archive it for future
> reference (if desired?)
> 

In order for this to be as intuitive and effective as possible, I 
suggest we stick with conventional usage. A single PR allows:

1. A single place to track all of the changes to the contribution 
including force pushes.

2. Clean (less cluttered) PR history where each unique contribution is 
represented by a single PR.

3. References to the PR such as comments, other PRs, and GitHub issues 
to automatically be reflected on the PR. This allows work related to the 
PR such as tracking issue items, future bug fix PRs, etc. to be easily 
found from the original PR.

The consistent PR number is important as it allows data queries that 
many other tools and GitHub workflows use to operate as expected. For 
example, many scripts and applications use the GitHub REST API [0] 
and/or GitHub GraphQL API [1] to query and present PR information. Logic 
would be complicated everywhere sorting through PRs that are dead ends 
to find if a PR is finally the last in a series, etc. Many off-the-shelf 
tools would produce incorrect results.

In addition, many tools and existing processes depend on a single PR 
link to track the status of a code change. Whether in public bug 
tracking tools or internal tools. That would produce stale links constantly.

It is possible to see force pushed diffs in a PR. For example, in this 
PR [2], I force pushed several times. You can simply click 
"force-pushed" or "Compare" on each force push to get a diff of the 
force pushed changes. On a simple rebase, as is the case in the last 
force push, the diff is empty and it reports the contents as identical.

With this it is possible to easily see branch (and its commit details) 
and checkout a specific push state using its respective commit hash.

[0] https://docs.github.com/en/rest?apiVersion=2022-11-28
[1] https://docs.github.com/en/graphql/overview
[2] https://github.com/tianocore/edk2/pull/4835

> [0] https://openfw.io/edk2-devel/
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118522): https://edk2.groups.io/g/devel/message/118522
Mute This Topic: https://groups.io/mt/105847510/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-