Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
> -Original Message- > From: r...@edk2.groups.io On Behalf Of Brian J. > Johnson > Sent: Thursday, May 2, 2024 8:59 AM > To: devel@edk2.groups.io; dionnagl...@google.com; > quic_llind...@quicinc.com > Cc: Kinney, Michael D ; r...@edk2.groups.io; > Andrew Fish (af...@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 5/1/24 18:19, Dionna Glaze via groups.io wrote: > > On Wed, May 1, 2024 at 11:12 AM Leif Lindholm via groups.io > > wrote: > >> > >> On 2024-05-01 18:43, Michael D Kinney 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. > >> > >> Thanks Mike. > >> > >> I'm in favour of this change, and the date. > >> > >> I still want us to try to figure out how to retain review history > beyond > >> what github decides we need, but I don't think it justifies > indefinitely > >> delaying the switchover. And frankly, it will be easier to experiment > >> with what works and not after the switch. > > > > +1. UI-based interactions don't export well for archival-permalinking > > reasons, and Github archive behavior is for repositories only, not the > > reviews. > > But yes, wouldn't want to delay for a bot to echo conversations to > > devel@edk2.groups.io or some other solution. > > > > +1 from me as well. We need to maintain review history in some fairly > permanent manner, both the reviewed code and review comments. > > >> > >> / > >> Leif > >> > >>> Updates to the following Wiki page would be required to describe the > >>> required process when using GitHub Pull Requests for all code review > >>> related activity. > >>> > >>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II- > Development-Process > >>> > >>> A couple examples of the changes that would need to be documented > are: > >>> > >>> * All contributors, maintainers, and reviewers must have GitHub IDs. > > > > It looks like this is already resolved for the existing > > Maintainers.txt with the `[username]` syntax, but I don't see any > > explanation of that expectation. Seems fine to me. > > > >>> * 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. > > Would those tags be optional? Test and ack info can be helpful when > researching a change, to find people who may be knowledgeable about it. > > Similarly, the Reviewed-by info is nice to have in the history, without > having to dig it out of archives. But it's a bit awkward to add on > github: you have to push new commits with the Reviewed-by tags, but > that changes the SHAs, so it's not obvious that the commits you're > merging have the same code as the ones which were reviewed. For the > email flow, we trust maintainers to get this right. For the github > flow, are we deciding to rely exclusively on the PR archives? > > What if a maintainer decides to tweak a commit before merging it, eg. to > fix a trivial typo? With the email flow they just go ahead and do it. > With the github flow, would they need to post another PR, so it could > make it through the process and be merged? > Editing commit messages after reviews is something we want to avoid so the PR does not need to be re-reviewed to be merged. GitHub has history of who did the reviews and the conversations that can include information on testing and review activity. Yes. Even trivial changes to the code would require the PR to be updated and re-reviewed. It is a different process than email where we allowed maintainers to make minor changes. However, even a maintainer making a minor change could break things in unexpected ways. It seems better to have stricter control over changes and all changes are reviewed and go through CI. > >>> * The Pull Request submitter is required to invite the required > >>> maintainers and reviewers to the pull request. This is the same > >>> set of maintainers and reviewers that are required to be listed > in > >>> Cc: tags in today's process.
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
> -Original Message- > From: r...@edk2.groups.io On Behalf Of Pedro Falcato > Sent: Thursday, May 2, 2024 10:51 AM > To: devel@edk2.groups.io; Kinney, Michael D > Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew Fish > (af...@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 > 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. > > > > Updates to the following Wiki page would be required to describe the > > required process when using GitHub Pull Requests for all code review > > related activity. > > > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II- > Development-Process > > > > A couple examples of the changes that would need to be documented are: > > > > * 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? I am curious how other GitHub projects handle this topic. I see it is possible with squash and merge to amend the commit message, but I do not see any features that allow all the commits in a series to be amended. TianoCore does not use squash and merge. The main reason to remove this edit of the commit messages is that the commit message edits look like a new version of the series to git/GitHub and requires review again. This extra edit is also a manual process and there is a history of maintainers missing who did the reviews and acks. The state in the GitHub PR is always accurate. > > * The Pull Request submitter is required to invite the required > > maintainers and reviewers to the pull request. This is the same > > set of maintainers and reviewers that are required to be listed in > > Cc: tags in today's process. > > * Maintainers are responsible for verifying that all conversations in > > the code review are resolved and that all review approvals from the > > required set of maintainers are present before setting the 'push' > label. > > > > > > Please provide feedback > > 1) If you are not in favor of this change. > > 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. > > -- > Pedro > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118539): https://edk2.groups.io/g/devel/message/118539 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On 5/2/2024 9:14 AM, Michael Kubacki wrote: [2] https://github.com/tianocore/edk2/pull/4835 I can't believe there's no UI for this, but in case anyone else is wondering: the diff can be downloaded by adding ".diff" or ".patch" onto the end of the URL - e.g. https://github.com/tianocore/edk2/pull/4835.diff Also, there's a command-line interface available via the 'gh' command - https://cli.github.com/ (see https://cli.github.com/manual/gh_pr for the PR related commands). -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118548): https://edk2.groups.io/g/devel/message/118548 Mute This Topic: https://groups.io/mt/105879875/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
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 wrote: > > > > > -Original Message- > > From: r...@edk2.groups.io On Behalf Of Pedro Falcato > > Sent: Thursday, May 2, 2024 10:51 AM > > To: devel@edk2.groups.io; Kinney, Michael D > > Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew Fish > > (af...@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 > > wrote: > > > * 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? > > 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 :) > > 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 (#118560): https://edk2.groups.io/g/devel/message/118560 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
Yes. Many options on transition. I would suggest we consider the following steps. 1) Define a manual process where Maintainers/Reviewers look for new PRs and make sure all the required maintainers/reviewers are invited to the review. Current process has many manual steps. Having this one does not seem like a bad transition option. 2) Add CODEOWNERS support to automate assignment of maintainers And enforce maintainer approval requirements. 3) Add REVIEWEWS support to automate assignment of reviewers. Mike > -Original Message- > From: r...@edk2.groups.io On Behalf Of Michael > Kubacki > Sent: Friday, May 3, 2024 10:22 AM > To: devel@edk2.groups.io; Kinney, Michael D > ; quic_llind...@quicinc.com; > marcin.juszkiew...@linaro.org; r...@edk2.groups.io > Cc: Leif Lindholm ; Andrew Fish (af...@apple.com) > > Subject: Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code > Review from email to GitHub Pull Requests on 5-24-2024 > > Hi Mike, > > I've seen that repo in that past. > > Are the steps defined for what's needed to move to CODEWONERS (and > REVIEWERS) in terms of technical and process changes needed? > > For example, could we start with CODEOWNERS manually synced to > Maintainers.txt, Maintainers.txt dropped, and then add REVIEWERS in the > future with additional actions to verify file coverage, etc.? > > Thanks, > Michael > > On 5/2/2024 12:24 PM, Michael D Kinney wrote: > > Hi Michael, > > > > I have a version of the auto assignment working, but needs to be > > migrated to TianoCore and synced with the latest Maintainers.txt. > > > > My experience getting this running even as a POC was that it took a > lot > > of effort to make sure the best security practices were followed and > > to configure the empty GitHub App with tokens and permissions. Anytime > > custom actions are added, resources to implement, validate, and > > support if they ever fail must be in place. > > > > My question is if there is a manual process that can be used to > > start and these type of automations can be added over time as > > dedicated resources are identified. > > > > Dionna's feedback about contributors not being able to add reviewers > > to a PR is correct. Contributors that are not members of the > > EDK II Maintainers or EDK II Reviewers teams will either need to wait > > for a Maintainer or Reviewer to add reviewers, or the contributor must > > be added as an "outside collaborator" by an admin. For a manual > process > > this would require Maintainers to monitor new PRs and make sure the > > correct set of Maintainers and Reviewers are added. Perhaps the > > contributor can include @GitHubId mentions in the PR for the required > > maintainers/reviewers so there are email notifications. > > > > Details on the auto assignment POC > > == > > It uses CODEOWNERS so maintainers are auto assigned and can use GitHub > > features to prevent merges without maintainer approval. The idea is > to > > minimize the custom behavior and use as many built-in GitHub features > as > > possible. It then reuses the CODEOWNERS syntax for a new file called > > REVIEWERS along with some GitHub Actions to auto assign reviewers. > The > > actions are run by a registered empty GitHub application so it has a > > TianoCore organization bot that is executing the actions with > TianoCore > > org permissions instead of an individual TianoCore member permissions. > > Example PR with the bot with the name "tianocore-assign-reviewers" > > performing assignments: > > https://github.com/tianocore/edk2-codereview/pull/91 > > > > It was activated on this repo to run experiments: > > https://github.com/tianocore/edk2-codereview/tree/master/.github > > > > Example CODEOWNERS file: > > https://github.com/tianocore/edk2- > codereview/blob/master/.github/CODEOWNERS > > > > Example REVIEWERS file using same format as CODEOWNERS > > https://github.com/tianocore/edk2- > codereview/blob/master/.github/REVIEWERS > > > > Action to assign reviewers from REVIEWERS file: > > https://github.com/tianocore/edk2- > codereview/blob/master/.github/workflows/AssignReviewers.yml > > > > Depends on: https://github.com/mdkinney/github-action-assign- > reviewers > > > > Action to verify that all files in a repo have CODEOWNES coverage > > https://github.com/tianocore/edk2- > codereview/blob/master/.github/workflows/CheckCodeOwnerFiles.yml > > > > Action to verify that Maintaine
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 > Sent: Friday, May 3, 2024 10:39 AM > To: Kinney, Michael D > Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm > ; Andrew Fish (af...@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 > wrote: > > > > > > > > > -Original Message- > > > From: r...@edk2.groups.io On Behalf Of Pedro > Falcato > > > Sent: Thursday, May 2, 2024 10:51 AM > > > To: devel@edk2.groups.io; Kinney, Michael D > > > > Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew > Fish > > > (af...@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 > > > wrote: > > > > > * 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/032830e96841f2a752e364378c3428ac5d2f59d1 Goto the "Pull Requests" tab for the repo and in the "Filters" search box enter is:pr is:merged 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 > > > > > 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 :) > > > > > 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 (#118563): https://edk2.groups.io/g/devel/message/118563 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
> -Original Message- > From: Kinney, Michael D > Sent: Friday, May 3, 2024 1:13 PM > To: Pedro Falcato > Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm > ; Andrew Fish (af...@apple.com) ; > Kinney, Michael D > 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 > > Sent: Friday, May 3, 2024 10:39 AM > > To: Kinney, Michael D > > Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm > > ; Andrew Fish (af...@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 > > wrote: > > > > > > > > > > > > > -Original Message- > > > > From: r...@edk2.groups.io On Behalf Of Pedro > > Falcato > > > > Sent: Thursday, May 2, 2024 10:51 AM > > > > To: devel@edk2.groups.io; Kinney, Michael D > > > > > > Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew > > Fish > > > > (af...@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 > > > > wrote: > > > > > > > * 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 > > 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 BRANCHCREATED AT #5560 Loongcpu niruiyu:loongcpu about 17 days ago > > > > > > > > > I am curious how other
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On 5/3/2024 4:38 PM, Michael D Kinney wrote: -Original Message- From: Kinney, Michael D Sent: Friday, May 3, 2024 1:13 PM To: Pedro Falcato Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm ; Andrew Fish (af...@apple.com) ; Kinney, Michael D 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 Sent: Friday, May 3, 2024 10:39 AM To: Kinney, Michael D Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm ; Andrew Fish (af...@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 wrote: -Original Message- From: r...@edk2.groups.io On Behalf Of Pedro Falcato Sent: Thursday, May 2, 2024 10:51 AM To: devel@edk2.groups.io; Kinney, Michael D Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew Fish (af...@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 wrote: * 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 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 BRANCHCREATED 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. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118566): https://edk2.groups.io/g/devel/message/118566 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On Sat, May 4, 2024 at 1:57 AM Michael Kubacki wrote: > > On 5/3/2024 4:38 PM, Michael D Kinney wrote: > > > > > >> -Original Message- > >> From: Kinney, Michael D > >> Sent: Friday, May 3, 2024 1:13 PM > >> To: Pedro Falcato > >> Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm > >> ; Andrew Fish (af...@apple.com) ; > >> Kinney, Michael D > >> 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 > >>> Sent: Friday, May 3, 2024 10:39 AM > >>> To: Kinney, Michael D > >>> Cc: r...@edk2.groups.io; devel@edk2.groups.io; Leif Lindholm > >>> ; Andrew Fish (af...@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 > >>> wrote: > >>>> > >>>> > >>>> > >>>>> -----Original Message- > >>>>> From: r...@edk2.groups.io On Behalf Of Pedro > >>> Falcato > >>>>> Sent: Thursday, May 2, 2024 10:51 AM > >>>>> To: devel@edk2.groups.io; Kinney, Michael D > >>> > >>>>> Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew > >>> Fish > >>>>> (af...@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 > >>>>> wrote: > >>> > >>>>>> * 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"
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
This reminds me: would it be possible to keep track of who merged a PR? (i.e., the person that set the 'push' label) Currently, commits just appear on the branch with the original author and the committer field set to something non-descript, e.g., commit 275d0a39c42ad73a6e4929822f56f5d8c16ede96 (HEAD -> master, origin/master, origin/HEAD) Author: Gerd Hoffmann AuthorDate: Fri Mar 1 08:44:00 2024 +0100 Commit: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> CommitDate: Fri Mar 1 18:47:27 2024 + which means we cannot tell from the git history which maintainer merged this. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118593): https://edk2.groups.io/g/devel/message/118593 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
That information is in GitHub in the PR conversation. If you follow the link from the commit to the PR, the PR conversation shows who set the 'push' label. Mike > -Original Message- > From: r...@edk2.groups.io On Behalf Of Ard > Biesheuvel > Sent: Monday, May 6, 2024 3:01 AM > To: devel@edk2.groups.io; Kinney, Michael D > Cc: Pedro Falcato ; r...@edk2.groups.io; Leif > Lindholm ; Andrew Fish (af...@apple.com) > > Subject: Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code > Review from email to GitHub Pull Requests on 5-24-2024 > > This reminds me: would it be possible to keep track of who merged a > PR? (i.e., the person that set the 'push' label) > > Currently, commits just appear on the branch with the original author > and the committer field set to something non-descript, e.g., > > commit 275d0a39c42ad73a6e4929822f56f5d8c16ede96 (HEAD -> master, > origin/master, origin/HEAD) > Author: Gerd Hoffmann > AuthorDate: Fri Mar 1 08:44:00 2024 +0100 > Commit: mergify[bot] > <37929162+mergify[bot]@users.noreply.github.com> > CommitDate: Fri Mar 1 18:47:27 2024 + > > which means we cannot tell from the git history which maintainer merged > this. > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118600): https://edk2.groups.io/g/devel/message/118600 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On Mon, 6 May 2024 at 17:11, Kinney, Michael D wrote: > > That information is in GitHub in the PR conversation. > > If you follow the link from the commit to the PR, the PR conversation shows > who set the 'push' label. > But that is GitHub proprietary metadata, no? Is it not possible to set the committer field in Git itself to something meaningful? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118601): https://edk2.groups.io/g/devel/message/118601 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
Hi Ard, Thais is an attribute of Mergify. I do not see a way to change that behavior. I do not know if using the GitHub merge capability or other merge services provides different behavior here or not. This specific request is not related to the change to GitHub PRs for code review. There is no intention to change the requirement for a maintainer to set the 'push' label and no intention to change away from Mergify at this time. Perhaps we can open your request as an independent request that we can find an owner to evaluate options and provide recommendations. Thanks, Mike > -Original Message- > From: Ard Biesheuvel > Sent: Monday, May 6, 2024 8:31 AM > To: Kinney, Michael D > Cc: r...@edk2.groups.io; devel@edk2.groups.io; Pedro Falcato > ; Leif Lindholm ; Andrew > Fish (af...@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 Mon, 6 May 2024 at 17:11, Kinney, Michael D > wrote: > > > > That information is in GitHub in the PR conversation. > > > > If you follow the link from the commit to the PR, the PR conversation > shows > > who set the 'push' label. > > > > But that is GitHub proprietary metadata, no? Is it not possible to set > the committer field in Git itself to something meaningful? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118602): https://edk2.groups.io/g/devel/message/118602 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On Mon, 6 May 2024 at 17:57, Kinney, Michael D wrote: > > Hi Ard, > > Thais is an attribute of Mergify. I do not see a way to change that > behavior. > > I do not know if using the GitHub merge capability or other merge services > provides different behavior here or not. > > This specific request is not related to the change to GitHub PRs > for code review. There is no intention to change the requirement > for a maintainer to set the 'push' label and no intention to change > away from Mergify at this time. > > Perhaps we can open your request as an independent request that we > can find an owner to evaluate options and provide recommendations. > Yep that works for me - it is not an urgent issue anyway, just something I realized while going through the thread. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118603): https://edk2.groups.io/g/devel/message/118603 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
Internal Use - Confidential On 2024-05-01 18:43, Michael D Kinney 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. Very welcome news! Make it so! William Leara BIOS Architect Dell | BIOS/FW Architecture office 512-720-5122 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#851): https://urldefense.com/v3/__https://edk2.groups.io/g/rfc/message/851__;!!LpKI!kLrsCBspYuhPcpzu6_oy_nM3IrLm1PLlibITwnmUZRndVxpkUNIpWNxB7UeJUaSNexPOL8sbHixsM2L7Yh5TnNdWdBU$ [edk2[.]groups[.]io] Mute This Topic: https://urldefense.com/v3/__https://groups.io/mt/105848092/7889204__;!!LpKI!kLrsCBspYuhPcpzu6_oy_nM3IrLm1PLlibITwnmUZRndVxpkUNIpWNxB7UeJUaSNexPOL8sbHixsM2L7Yh5TxZFqh5M$ [groups[.]io] Group Owner: rfc+ow...@edk2.groups.io Unsubscribe: https://urldefense.com/v3/__https://edk2.groups.io/g/rfc/unsub__;!!LpKI!kLrsCBspYuhPcpzu6_oy_nM3IrLm1PLlibITwnmUZRndVxpkUNIpWNxB7UeJUaSNexPOL8sbHixsM2L7Yh5T3vhXcBU$ [edk2[.]groups[.]io] [william.le...@dell.com] -=-=-=-=-=-=-=-=-=-=-=- -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118660): https://edk2.groups.io/g/devel/message/118660 Mute This Topic: https://groups.io/mt/105977012/21656 Group Owner: develꌉ@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
On 5/3/24 12:38, Pedro Falcato wrote: On Thu, May 2, 2024 at 7:17 PM Kinney, Michael D wrote: -Original Message- From: r...@edk2.groups.io On Behalf Of Pedro Falcato Sent: Thursday, May 2, 2024 10:51 AM To: devel@edk2.groups.io; Kinney, Michael D Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew Fish (af...@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 wrote: * 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!) 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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] Proposal to switch TianoCore Code Review from email to GitHub Pull Requests on 5-24-2024
> -Original Message- > From: Brian J. Johnson > Sent: Friday, May 10, 2024 1:58 PM > To: r...@edk2.groups.io; pedro.falc...@gmail.com; Kinney, Michael D > > Cc: devel@edk2.groups.io; Leif Lindholm ; Andrew Fish > (af...@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 5/3/24 12:38, Pedro Falcato wrote: > > On Thu, May 2, 2024 at 7:17 PM Kinney, Michael D > > wrote: > >> > >> > >> > >>> -Original Message- > >>> From: r...@edk2.groups.io On Behalf Of Pedro Falcato > >>> Sent: Thursday, May 2, 2024 10:51 AM > >>> To: devel@edk2.groups.io; Kinney, Michael D > >>> Cc: r...@edk2.groups.io; Leif Lindholm ; Andrew Fish > >>> (af...@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 > >>> wrote: > > > >>>> * 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 There are github actions that can archive this information in a portable format such as json: https://github.com/marketplace/actions/github-archive-action > > > 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%2FBu > s%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!) Can you propose a process to provide review comments on the commit messages within a PR conversation? Perhaps use of keywords/tags/links to indicate the commit being discussed? Could copy/paste commit message being reviewed into PR conversation and provide feedback there. > > > > >>> 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 (#118927): https://edk2.groups.io/g/devel/message/118927 Mute This Topic: https://groups.io/mt/105873467/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
