Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Dong, Eric <eric.dong@...>
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brian J. Johnson
Sent: Saturday, June 20, 2020 1:29 AM To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@...>; rfc@edk2.groups.io Cc: Dong, Eric <eric.dong@...>; Ni, Ray <ray.ni@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Tan, Ming <ming.tan@...> Subject: Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules On 6/18/20 2:01 AM, Dandan Bi wrote: Hi All, REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2816 We plan to separate two kinds of NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler/.../ StatusCodeHandlerPei/RuntimeDxe/Smm modules. The benefit we want to gain from this separation is to 1) make the code clear and easy to maintain, 2) make platform flexible to choose any handler library they need, and it also can reduce image size since the unused handlers can be excluded. If you have any concern or comments for this separation, please let me know. We plan to add new separated NULL class library MemoryStausCodeHandlerLib and SerialStatusCodeHandlerLib with different phase implementation into MdeModulePkg\Library\ directory. The main tree structure may like below: MdeModulePkg\Library |------MemoryStausCodeHandlerLib |------|------ PeiMemoryStausCodeHandlerLib.inf |------|------ RuntimeDxeMemoryStatusCodeHandlerLib.inf |------|------ SmmMemoryStausCodeHandlerLib.inf |------SerialStatusCodeHandlerLib |------|------ PeiSerialStatusCodeHandlerLib.inf |------|------ RuntimeDxeSerialStatusCodeHandlerLib.inf |------|------ SmmSerialStatusCodeHandlerLib.inf We will update existing platform use cases in edk2 and edk2-platform repo to cover the new NULL class library to make sure this change doesn't impact any platform. After this separation, StatusCodeHandler module usage will like below, and it's also very flexible for platform to cover more handler libraries to meet their requirements. MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf { <LibraryClasses> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf ... } MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf { <LibraryClasses> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf ... } MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf { <LibraryClasses> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemoryStausCodeHandlerLib.inf NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatusCodeHandlerLib.inf ... } Thanks, Dandan Dandan, We'll have a lot of layers of indirection.... The ReportStatusCodeRouter modules will call one or more StatusCodeHandlerModules, and the standard StatusCodeHandler modules will call multiple StatusCodeHandlerLib libraries. How about adding StatusCodeHandlerLib support directly to the ReportStatusCodeRouter modules? Then platforms could omit the StatusCodeHandler modules if they're only using the open-source code. That sounds like less overhead since fewer modules would be needed Hi Brain, You are right. Current design truly has a lot of layers. The ReportStatusCodeRouter module provides the register logic and maintain the registered status code handlers. Now the platform may have more than one of drivers used to register the status code handler. This RFC used to resolve the platform has more than one status code handler drivers' issue. We expect the platform only need one wrapper driver in MdeModulePkg to let the status code handler library to register its handler on it. Thanks, Eric Thanks, -- Brian J. Johnson Enterprise X86 Lab Hewlett Packard Enterprise hpe.com<3D%22hpe.com%22>
|
|
Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Andrew Fish <afish@...>
On Jun 19, 2020, at 10:29 AM, Brian J. Johnson <brian.johnson@...> wrote:I think the need to execute from ROM makes this tricky. It looks to me that it is easy to move from PCD to libs for the StatusCodeHandler since registration is basically `RscHandlerPpi->Register (SerialStatusCodeReportWorker);`. The issue I see is the ReportStatusCodeRouter publishes RscHandlerPpi after the PEIMs constructor has been called and if the PEIM. Given globals don’t work when running from ROM you would have to do something like publish a HOB in the library constructor and then have the GenericStatusCodePeiEntry() walk the HOBs and install the handlers. So I guess it is a little easier than I 1st thought when I started writing this mail, but it would require a new public API. Thanks, Andrew Fish Thanks,
|
|
Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Brian J. Johnson
On 6/18/20 2:01 AM, Dandan Bi wrote:
Dandan, We'll have a lot of layers of indirection.... The ReportStatusCodeRouter modules will call one or more StatusCodeHandlerModules, and the standard StatusCodeHandler modules will call multiple StatusCodeHandlerLib libraries. How about adding StatusCodeHandlerLib support directly to the ReportStatusCodeRouter modules? Then platforms could omit the StatusCodeHandler modules if they're only using the open-source code. That sounds like less overhead since fewer modules would be needed. Thanks, -- *Brian J. Johnson *Enterprise X86 Lab Hewlett Packard Enterprise *hpe.com* <3D"hpe.com">
|
|
Re: [edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Laszlo Ersek
On 06/18/20 09:01, Dandan Bi wrote:
Hi All,So I assume you're going to remove PcdStatusCodeUseSerial and PcdStatusCodeUseMemory, and when converting the existent platforms, the new NULL class resolutions in the DSC files will reflect the specific PCD values used in those DSC files until then. Is that right? I'm OK with it. Thanks Laszlo
|
|
[edk2-devel] [edk2-rfc] MdeModulePkg/StatusCodeHandler: Separate NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler modules
Dandan Bi <dandan.bi@...>
Hi All,
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2816 We plan to separate two kinds of NULL class libraries for Memory and serial handlers from MdeModulePkg/Universal/StatusCodeHandler/.../ StatusCodeHandlerPei/RuntimeDxe/Smm modules. The benefit we want to gain from this separation is to 1) make the code clear and easy to maintain, 2) make platform flexible to choose any handler library they need, and it also can reduce image size since the unused handlers can be excluded. If you have any concern or comments for this separation, please let me know. We plan to add new separated NULL class library MemoryStausCodeHandlerLib and SerialStatusCodeHandlerLib with different phase implementation into MdeModulePkg\Library\ directory. The main tree structure may like below: MdeModulePkg\Library |------MemoryStausCodeHandlerLib |------|------ PeiMemoryStausCodeHandlerLib.inf |------|------ RuntimeDxeMemoryStatusCodeHandlerLib.inf |------|------ SmmMemoryStausCodeHandlerLib.inf |------SerialStatusCodeHandlerLib |------|------ PeiSerialStatusCodeHandlerLib.inf |------|------ RuntimeDxeSerialStatusCodeHandlerLib.inf |------|------ SmmSerialStatusCodeHandlerLib.inf We will update existing platform use cases in edk2 and edk2-platform repo to cover the new NULL class library to make sure this change doesn't impact any platform. After this separation, StatusCodeHandler module usage will like below, and it's also very flexible for platform to cover more handler libraries to meet their requirements. MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf { <LibraryClasses> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/PeiMemoryStausCodeHandlerLib.inf NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/PeiSerialStatusCodeHandlerLib.inf ... } MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf { <LibraryClasses> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/RuntimeDxeMemoryStausCodeHandlerLib.inf NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/RuntimeDxeSerialStatusCodeHandlerLib.inf ... } MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf { <LibraryClasses> NULL|MdeModulePkg/Library/MemoryStausCodeHandlerLib/SmmMemoryStausCodeHandlerLib.inf NULL|MdeModulePkg/Library/SerialStatusCodeHandlerLib/SmmSerialStatusCodeHandlerLib.inf ... } Thanks, Dandan
|
|
Re: [edk2-devel] [edk2-rfc] [RFCv2] code-first process for UEFI-forum specifications
Samer El-Haj-Mahmoud
Leif,
toggle quoted messageShow quoted text
I received additional feedback on this proposal. We should add the UEFI Shell Specification to this new process. This includes adding a bugzilla.tianocore.org product category and a new Github repository for the "UEFI Shell Specification". Thanks, --Samer
-----Original Message-----IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Bret Barkelew <bret.barkelew@...>
Rebecca,
I was able to confirm that it was an issue with Git for Windows. Looks like it’s fixed in current snapshots and will be in the next release: https://github.com/git-for-windows/git/issues/2598 Also, ATTN: @Michael Kubacki<mailto:Michael.Kubacki@...> - Bret From: Bret Barkelew<mailto:Bret.Barkelew@...> Sent: Wednesday, May 27, 2020 10:45 AM To: Rebecca Cran<mailto:rebecca@...>; rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; lersek@...<mailto:lersek@...>; Andrew Fish<mailto:afish@...> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; spbrogan@...<mailto:spbrogan@...>; Desimone, Nathaniel L<mailto:nathaniel.l.desimone@...>; Kinney, Michael D<mailto:michael.d.kinney@...>; Leif Lindholm (Nuvia address)<mailto:leif@...> Subject: RE: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process That’s not a bad idea: I should try with my WSL install. I’m on the same version of Git for Windows, and think I’ll open it as an issue to the maintainer. For now, going though the paces is just as useful to me as getting a viable environment (after all, PRs soon!), so I don’t mind trying another OS or install if that’s what it takes. - Bret From: Rebecca Cran<mailto:rebecca@...> Sent: Wednesday, May 27, 2020 9:07 AM To: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; lersek@...<mailto:lersek@...>; Bret Barkelew<mailto:Bret.Barkelew@...>; Andrew Fish<mailto:afish@...> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; spbrogan@...<mailto:spbrogan@...>; Desimone, Nathaniel L<mailto:nathaniel.l.desimone@...>; Kinney, Michael D<mailto:michael.d.kinney@...>; Leif Lindholm (Nuvia address)<mailto:leif@...> Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process On 5/27/2020 6:12 AM, Laszlo Ersek wrote: So, it could be a MINGW64 packaging bug, perhaps.I'm getting the same error, but with a different packaging of Git: mine's in C:\Program Files\Git\cmd\git.exe . It's version "git version 2.26.2.windows.1". Of course it's possible it's just the same MINGW version that's been put into its own installer. I also tried using my openSUSE WSL installation, but it failed with: STARTTLS failed! SSL connect attempt failed error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed at /usr/lib/git/git-send-email line 1548. So I ended up copying it to one of my FreeBSD systems and sent it from there. -- Rebecca Cran
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Laszlo Ersek
On 05/28/20 00:07, Rebecca Cran wrote:
I also tried using my openSUSE WSL installation, but it failed with:That's different -- in this case, peer certificate verification was attempted, but it failed, because the root certificate in the peer's cert chain is not trusted by your system (your openSUSE WSL environment). The fix for that should be identical to what you'd do on a standalone openSUSE installation -- (1) figure out what CA cert is the root of the peer's cert chain, and (2) decide consciously whether you trust that CA cert to sign other certificates, (3) import said CA cert persistently into your "store of trusted CA certs". Examples: (1) I think one command that works is: $ openssl s_client -showcerts -connect HOST:PORT </dev/null (2) up to you :) (3a) On RHEL, this would mean copying the CA certificate under "/etc/pki/ca-trust/source/anchors/", in PEM format, and then running the "update-ca-trust extract" command. (Both actions need root (uid=0) access, of course.) (3b) For a user session (i.e., not system-wide), git-send-email also takes "--smtp-ssl-cert-path": --smtp-ssl-cert-path Path to a store of trusted CA certificates for SMTP SSL/TLS certificate validation (either a directory that has been processed by c_rehash, or a single file containing one or more PEM format certificates concatenated together: see verify(1) -CAfile and -CApath for more information on these). Set it to an empty string to disable certificate verification. Defaults to the value of the sendemail.smtpsslcertpath configuration variable, if set, or the backing SSL library's compiled-in default otherwise (which should be the best choice on most platforms). Thanks Laszlo
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Bret Barkelew <bret.barkelew@...>
That’s not a bad idea: I should try with my WSL install.
I’m on the same version of Git for Windows, and think I’ll open it as an issue to the maintainer. For now, going though the paces is just as useful to me as getting a viable environment (after all, PRs soon!), so I don’t mind trying another OS or install if that’s what it takes. - Bret From: Rebecca Cran<mailto:rebecca@...> Sent: Wednesday, May 27, 2020 9:07 AM To: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; lersek@...<mailto:lersek@...>; Bret Barkelew<mailto:Bret.Barkelew@...>; Andrew Fish<mailto:afish@...> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; spbrogan@...<mailto:spbrogan@...>; Desimone, Nathaniel L<mailto:nathaniel.l.desimone@...>; Kinney, Michael D<mailto:michael.d.kinney@...>; Leif Lindholm (Nuvia address)<mailto:leif@...> Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process On 5/27/2020 6:12 AM, Laszlo Ersek wrote: So, it could be a MINGW64 packaging bug, perhaps.I'm getting the same error, but with a different packaging of Git: mine's in C:\Program Files\Git\cmd\git.exe . It's version "git version 2.26.2.windows.1". Of course it's possible it's just the same MINGW version that's been put into its own installer. I also tried using my openSUSE WSL installation, but it failed with: STARTTLS failed! SSL connect attempt failed error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed at /usr/lib/git/git-send-email line 1548. So I ended up copying it to one of my FreeBSD systems and sent it from there. -- Rebecca Cran
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Andrew Fish <afish@...>
Rebecca,
toggle quoted messageShow quoted text
I cheated and used smtpServer = relay.apple.com and smtpEncryption = tls. Seems relay.apple.com does not require authentication and it just worked. I used an internal git mailing list to figure all this out, but seems the easy button was an smtpServer setup to be easy to use with git sendmail. It might be worth while to have folks reach out inside their companies to see if there are existing known good recipes? Thanks, Andrew Fish
On May 27, 2020, at 3:07 PM, Rebecca Cran <rebecca@...> wrote:
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Rebecca Cran
On 5/27/2020 6:12 AM, Laszlo Ersek wrote:
So, it could be a MINGW64 packaging bug, perhaps.I'm getting the same error, but with a different packaging of Git: mine's in C:\Program Files\Git\cmd\git.exe . It's version "git version 2.26.2.windows.1". Of course it's possible it's just the same MINGW version that's been put into its own installer. I also tried using my openSUSE WSL installation, but it failed with: STARTTLS failed! SSL connect attempt failed error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed at /usr/lib/git/git-send-email line 1548. So I ended up copying it to one of my FreeBSD systems and sent it from there. -- Rebecca Cran
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Laszlo Ersek
On 05/27/20 03:52, Bret Barkelew wrote:
So, today I followed the Wiki (that I had never seen) and now I’m staring down the barrel of this fellow…I think your perl installation and your git installation may come from different sources, and the perl install may not satisfy the git install's dependencies. In GNU/Linux distribution lingo, I'd call this either a distribution error, or (maybe more precisely) a git package error. Normally the git package should spell out the pre-requisite package names, along with the minimum required package version(s). And the package manager should enforce that, when installing git. Other people appear to have encountered a similiar issue before: https://github.com/Homebrew/homebrew-core/issues/24210#issuecomment-366831944 https://github.com/msys2/MSYS2-packages/issues/1152 https://bugs.archlinux.org/task/54326 https://bugs.archlinux.org/task/62948 When I check on my laptop now, I see: $ rpm --query --requires git-email [...] perl(Net::SMTP::SSL) [...] and recursively, $ rpm --query --requires perl-Net-SMTP-SSL [...] perl(IO::Socket::SSL) [...] In other words, whenever it was that I ran "yum install git-email" (for gaining access to the "git-send-email" command), yum made sure that "perl-Net-SMTP-SSL" and "perl-IO-Socket-SSL" would both be pulled in. (Assuming the RPM spec files spelled out minimum versions on the dependencies, yum would enforce those particular versions too.) So, it could be a MINGW64 packaging bug, perhaps. Thanks, Laszlo
|
|
Re: PKCS7 Authenticated Variable Enrollment
Guomin Jiang
I am sorry that I have not the use case, and I plan to investigating it after August.
toggle quoted messageShow quoted text
-----Original Message-----
|
|
Re: PKCS7 Authenticated Variable Enrollment
Wadhawan, Divneil R
Hi Goumin,
I had discussion internally, and I got hold off tools from: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/efitools.git. It is generating the correct format as per EFI_VARIABLE_AUTHENTICATION_2. So, I thought of first validating RSA2048 Sign verification and it is failing. I still have to figure out that. Do you have a working use case which uses PKCS7 format and PKCS7_verify works? Regards, Divneil
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Tomas Pilar (tpilar)
This will probably be down to the [send-email] section of git config, do you have smtpEncryption enabled by any chance?
You could also try updating the required package: perl -MCPAN -e 'install "IO::Socket::SSL"' From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io Sent: 27 May 2020 02:53 To: Andrew Fish <afish@...>; Laszlo Ersek <lersek@...> Cc: devel@edk2.groups.io; spbrogan@...; rfc@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Kinney, Michael D <michael.d.kinney@...>; Leif Lindholm (Nuvia address) <leif@...> Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process So, today I followed the Wiki (that I had never seen) and now I'm staring down the barrel of this fellow... [cid:image001.png@...] [Not using SSL_VERIFY_PEER due to out-of-date IO::Socket::SSL. To use SSL please install IO::Socket::SSL with version>=2.007 at /usr/share/perl5/core_perl/Net/SMTP.pm line 270.] Anyone have thoughts? I'mma go get a scotch. - Bret From: Andrew Fish<mailto:afish@...> Sent: Monday, May 25, 2020 11:28 AM To: Laszlo Ersek<mailto:lersek@...> Cc: Bret Barkelew<mailto:Bret.Barkelew@...>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; spbrogan@...<mailto:spbrogan@...>; rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; Desimone, Nathaniel L<mailto:nathaniel.l.desimone@...>; Kinney, Michael D<mailto:michael.d.kinney@...>; Leif Lindholm (Nuvia address)<mailto:leif@...> Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process On May 25, 2020, at 11:10 AM, Laszlo Ersek <lersek@...<mailto:lersek@...>> wrote:Laszlo, Your wiki article was very very helpful. I just could not find it from the Tianocre wiki. It would be good if we could link to it from here [1], maybe as add to this: "Are you new to using git? If so, then the New to git page may be helpful."? There are a lot folks who use git but don't use the email based review so they have never setup git with emali before. Your wiki, plus me figuring out the magic internal SMTP reflector (I reached out on an internal git malling list) is what got me unblocked. [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Development-Process&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2e084613c24f433ca0a508d800d978de%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260281325061578&sdata=XPx6jrloPC9LW0iCecZuFmaz3JgjCSQYeF0PEyGW4I0%3D&reserved=0 Thanks, Andrew Fish Thanks IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Bret Barkelew <bret.barkelew@...>
So, today I followed the Wiki (that I had never seen) and now I’m staring down the barrel of this fellow…
[cid:image002.png@...] [Not using SSL_VERIFY_PEER due to out-of-date IO::Socket::SSL. To use SSL please install IO::Socket::SSL with version>=2.007 at /usr/share/perl5/core_perl/Net/SMTP.pm line 270.] Anyone have thoughts? I’mma go get a scotch. - Bret From: Andrew Fish<mailto:afish@...> Sent: Monday, May 25, 2020 11:28 AM To: Laszlo Ersek<mailto:lersek@...> Cc: Bret Barkelew<mailto:Bret.Barkelew@...>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; spbrogan@...<mailto:spbrogan@...>; rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; Desimone, Nathaniel L<mailto:nathaniel.l.desimone@...>; Kinney, Michael D<mailto:michael.d.kinney@...>; Leif Lindholm (Nuvia address)<mailto:leif@...> Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process On May 25, 2020, at 11:10 AM, Laszlo Ersek <lersek@...> wrote:Laszlo, Your wiki article was very very helpful. I just could not find it from the Tianocre wiki. It would be good if we could link to it from here [1], maybe as add to this: "Are you new to using git? If so, then the New to git page may be helpful."? There are a lot folks who use git but don't use the email based review so they have never setup git with emali before. Your wiki, plus me figuring out the magic internal SMTP reflector (I reached out on an internal git malling list) is what got me unblocked. [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Development-Process&data=02%7C01%7CBret.Barkelew%40microsoft.com%7C2e084613c24f433ca0a508d800d978de%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260281325061578&sdata=XPx6jrloPC9LW0iCecZuFmaz3JgjCSQYeF0PEyGW4I0%3D&reserved=0 Thanks, Andrew Fish Thanks
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Bret Barkelew <bret.barkelew@...>
Samer,
Have you had a chance to review Mike’s PR process? Any thoughts as comparison? - Bret ________________________________ From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Samer El-Haj-Mahmoud via groups.io <samer.el-haj-mahmoud@...> Sent: Tuesday, May 26, 2020 7:39:55 AM To: rfc@edk2.groups.io <rfc@edk2.groups.io>; lersek@... <lersek@...>; Andrew Fish <afish@...> Cc: Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io <devel@edk2.groups.io>; spbrogan@... <spbrogan@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Kinney, Michael D <michael.d.kinney@...>; Leif Lindholm (Nuvia address) <leif@...>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...> Subject: Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process I agree with Andrew. I also found Laszlo's "unkempt guide" very useful. In addition, there is a short page by Peter Batard that adds more details on the commits validation, patchset generation, and e-mail submission: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgist.github.com%2Fpbatard%2Fec1c9d1dd6e7144b07a09b057b1735a8&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cdca587d1198049354a6f08d80182b15a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637261008123224059&sdata=e%2Bk1gQubOWY8gWlQAUAmjIIaqQMv6p%2FMqjUHcntVm1g%3D&reserved=0 -----Original Message-----IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
|
Re: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Tomas Pilar (tpilar)
I actually agree with you, when we migrated from reviewboard to github pull requests, I was sorely disappointed with the PR functionality and ergonomics.
toggle quoted messageShow quoted text
Tomas Pilar
-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Rebecca Cran via groups.io Sent: 14 May 2020 22:47 To: rfc@edk2.groups.io; bret.barkelew@...; Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; lersek@... Subject: Re: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process On 5/14/20 3:26 PM, Bret Barkelew via groups.io wrote: I feel like this process is a good compromise. It�s not perfect (frankly, I�m a fan of enforced squash merges, which can maintain bisectability if managed well), but it allows for rapid iteration, ease of contribution, and approaches the workflow that many who have never used email to maintain a project would be familiar with.Or at least, that's what Microsoft is betting on! :D Personally, I remain unconvinced about the usability of Github Pull Requests for a project the size of EDK2, but I hope to be proven wrong. -- Rebecca Cran IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Samer El-Haj-Mahmoud
I agree with Andrew. I also found Laszlo's "unkempt guide" very useful. In addition, there is a short page by Peter Batard that adds more details on the commits validation, patchset generation, and e-mail submission: https://gist.github.com/pbatard/ec1c9d1dd6e7144b07a09b057b1735a8
toggle quoted messageShow quoted text
-----Original Message-----IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
|
Re: [EXTERNAL] [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process
Laszlo Ersek
On 05/25/20 20:28, Andrew Fish wrote:
The article at [1] is an official document, while the "unkempt guide" isOn May 25, 2020, at 11:10 AM, Laszlo Ersek <lersek@...> wrote:Laszlo, not official. The unkempt guide starts by deferring to [1]. I didn't think the official document should point to my unofficial one, and/or we should create a loop of links. That said, if someone else updates [1] with a pointer, I won't protest. That's just something that I (having authored the unkempt guide) would not propose myself. I do agree that the wiki search facilities on github are basic. What has mostly worked for me is clicking the Pages arrow, and then entering a *very simple* search term in the drop-down search box. For example, if I do that now, and only enter "git", then the "unkempt guide" is listed (with other hits of course). I think this search box is basically for searching article titles. It's great that you have access to such infrastructure at Apple! Thanks! Laszlo
|
|