回复: [edk2-devel] more development process failure [was: UefiPayloadPkg: Runtime MMCONF]

Thu Sep 17 01:49:08 UTC 2020

Guo:
On pull request, https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-maintainer-process-for-the-edk-ii-project section 7 gives the requirement. 
If <new-integration-branch> is a patch series, then copy the patch #0 summary into the pull request description.

Laszlo:
I think we can enhance wiki page https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-maintainer-process-for-the-edk-ii-project to add another step to reply the patch mail with the merged pull request or commit after PR is merged. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+65339+4905953+8761045 at groups.io
> <bounce+27952+65339+4905953+8761045 at groups.io> 代表 Laszlo Ersek
> 发送时间: 2020年9月17日 2:14
> 收件人: Dong, Guo <guo.dong at intel.com>; devel at edk2.groups.io
> 抄送: marcello.bauer at 9elements.com; Kinney, Michael D
> <michael.d.kinney at intel.com>; Leif Lindholm (Nuvia address)
> <leif at nuviainc.com>; Doran, Mark <mark.doran at intel.com>; Andrew Fish
> <afish at apple.com>; Guptha, Soumya K <soumya.k.guptha at intel.com>
> 主题: Re: [edk2-devel] more development process failure [was:
> UefiPayloadPkg: Runtime MMCONF]
> 
> On 09/16/20 19:30, Dong, Guo wrote:
> >
> > Hi Laszlo,
> >
> > The patchset includes 3 patches, and all of them had been reviewed by
> package owners.
> > The patch submitter has a pull request
> https://github.com/tianocore/edk2/pull/885, I rebased the patch to latest
> master, and merged it by adding reviewed-by found from emails.
> > I also make sure it passed all the checks before I put "push" button there.
> then retrigger a new build with "push" button.
> >
> > I am not sure what is missing. If there is any other requirements, should
> they be captured during code review or tool check?
> 
> - The description field of <https://github.com/tianocore/edk2/pull/932/>
> is empty. It's difficult to tell where the patches come from -- where
> they were posted and reviewed. A copy of the cover letter should have
> been included here, plus preferably a link to the v5 mailing list thread
> (the one that got merged in the end).
> 
> - It was not confirmed in the v5 mailing list thread that the series had
> been merged. The confirmation should have included at least one of: (a)
> the github PR link, (b) the git commit range. (Preferably: both.)
> 
> It's not the eventual git commits that I'm complaining about, but the
> lack of communication with the community, and the lack of record for
> posterity.
> 
> Myself, I used to consider github PRs a means merely for replacing our
> earlier direct "git push" commands -- with a CI build + mergify. So, as
> a maintainer, I would myself queue up several patch sets in a single
> "batch" PR, add some links to BZs and the mailing list, and let it fly.
> But then Mike told me this was really wrong, and we should clearly
> associate any given PR with a specific patch set on the list.
> 
> This meant an *immense* workload increase for me, in particular because
> I tend to merge patch sets for *other* people and subsystems too (after
> they pass review), that is, for such subsystems that I do not
> co-maintain. In particular during the feature freeze periods.
> 
> So what really rubs me the wrong way is that, if I am expected to keep
> all of this meta-data nice and tidy, why aren't some other maintainers?
> It's a double standard.
> 
> I can live with either *all of us* ignoring PR tidiness, or *all of us*
> doing our best to keep everything nicely cross-referenced.
> 
> But right now I spend significant time and effort on keeping
> communication and records complete and clean in *all three of* bugzilla,
> github, and mailing list, whereas a good subset of the maintainers
> couldn't care less in *either* of those communication channels.
> 
> For your reference, here's a random PR I submitted and merged for others:
> 
>   https://github.com/tianocore/edk2/pull/904
> 
> Observe in PR#904:
> 
> - title carries cover letter subject
> - description carries cover letter body
> - description has a pointer to the BZ, and a link to the cover letter in
> the mailing list archive (two links in fact, in different archives)
> 
> And then here's my report back on the list:
> 
>   https://edk2.groups.io/g/devel/message/64644
> 
> And my BZ comment to the same effect (also closing the BZ as
> RESOLVED|FIXED):
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=2376#c9
>   https://edk2.groups.io/g/bugs/message/12777
> 
> 
> I don't insist on the particular information content of github PRs, as
> -- at this stage -- they really are not more than just a way to set off
> CI, before pushing/merging a series.
> 
> What I do insist on is that all of us maintainers (people with
> permission to set the "push" label) be subject to the same expectations
> when it comes to creating pull requests.
> 
> (Please note also that I absolutely don't need a BZ for every
> contribution. My request is only that *if* there is a BZ, then handle it
> thoroughly.)
> 
> Laszlo
> 
> 
> >
> > Thanks,
> > Guo
> >
> >> -----Original Message-----
> >> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Laszlo
> >> Ersek
> >> Sent: Wednesday, September 16, 2020 1:57 AM
> >> To: Dong, Guo <guo.dong at intel.com>
> >> Cc: devel at edk2.groups.io; marcello.bauer at 9elements.com; Kinney,
> Michael D
> >> <michael.d.kinney at intel.com>; Leif Lindholm (Nuvia address)
> >> <leif at nuviainc.com>; Doran, Mark <mark.doran at intel.com>; Andrew Fish
> >> <afish at apple.com>; Guptha, Soumya K <soumya.k.guptha at intel.com>
> >> Subject: [edk2-devel] more development process failure [was:
> UefiPayloadPkg:
> >> Runtime MMCONF]
> >>
> >> Guo,
> >>
> >> On 08/18/20 10:24, Marcello Sylvester Bauer wrote:
> >>> Support arbitrary platforms with different or even no MMCONF space.
> >>> Fixes crash on platforms not exposing 256 buses.
> >>>
> >>> Tested on:
> >>> * AMD Stoney Ridge
> >>>
> >>> Branch: https://github.com/9elements/edk2-1/tree/UefiPayloadPkg-
> >> MMCONF
> >>> PR: https://github.com/tianocore/edk2/pull/885
> >>>
> >>> v5:
> >>> * MdePkg
> >>>   - support variable size MMCONF in all PciExpressLibs
> >>>   - use (UINTX)-1 as return values for invalid Pci addresses
> >>
> >> Okay, so we got more of the same development process violations here, as
> >> I've just reported at <https://edk2.groups.io/g/devel/message/65313>.
> >>
> >> See this new pull request:
> >>
> >>   https://github.com/tianocore/edk2/pull/932/
> >>
> >> "No description provided."
> >>
> >> You should be embarrassed.
> >>
> >> Laszlo
> >>
> >>
> >>
> >>
> >>
> >
> 
> 
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65348): https://edk2.groups.io/g/devel/message/65348
Mute This Topic: https://groups.io/mt/76901786/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-