On 5/2/2024 5:37 AM, Ard Biesheuvel wrote:
On Wed, 1 May 2024 at 19:44, Michael D Kinney
<michael.d.kin...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to