Thanks for your explanation and fastidiousness, Laszlo. Much appreciated. On Thu, Jan 12, 2023 at 7:32 AM Laszlo Ersek <ler...@redhat.com> wrote: > > On 1/12/23 14:38, Ard Biesheuvel wrote: > > On Thu, 12 Jan 2023 at 13:24, Laszlo Ersek <ler...@redhat.com> wrote: > >> > >> On 1/12/23 00:08, Dionna Glaze via groups.io wrote: > >>> On Thu, Nov 10, 2022 at 8:55 AM Kinney, Michael D > >>> <michael.d.kin...@intel.com> wrote: > >>>> > >>>> Hi Dionna, > >>>> > >>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > >>>> > >>>> I think you are at step 13. If you have not done so already, > >>>> please update the commit messages with all the Reviewed-by and > >>>> Acked-by tags and make sure your branch is rebased against the > >>>> latest edk2/master and update the PR with that content and verify > >>>> that all EDK II CI checks pass. > >>>> > >>>> At that point, it is the responsibility of the EDK II Maintainers > >>>> to review the final state of the PR and set the "push" label if > >>>> everything looks correct. > > I didn't realize this had been adopted -- "upgrading" a contributor > submitted PR. (More below.) > > >>>> At this moment, we are in Soft Freeze and will be in Hard Freeze > >>>> for the next 2 weeks. If this is considered critical for the > >>>> stable tag release, then please send a request to Liming with > >>>> justification for stable tag. Otherwise, it will be merged after > >>>> the stable tag release. > >>>> > >>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning > >>>> > >>>> Thanks, > >>>> > >>>> Mike > >>>> > >>> > >>> Hi Mike, my PR was closed without comment > >>> https://github.com/tianocore/edk2/pull/3623 so I rebased and created > >>> a new PR after the holidays and hard freeze. I hope this isn't > >>> considered bad practice https://github.com/tianocore/edk2/pull/3885 > >>> > >> > >> I closed the PR -- similarly to how I manually closed 400+ stale PRs > >> then -- because it was not a maintainer-submitted PR with the "push" > >> label set. In other words, it was just a personal build, for > >> triggering a CI run. > >> > >> And, in fact regardlessly of whether it was a "push"-labeled > >> maintainer PR or just a personal PR, the PR was almost two months > >> old. Clearly stale and abandoned -- per edk2 contribution workflow, > >> anyway. > >> > >> Please excuse me for not explaining this in a comment on the PR, but > >> (as I said above) I closed 400+ stale PRs manually within 10-15 > >> minutes on the github.com WebUI. The necessary muscle memory training > >> didn't allow for individual comments. > >> > >> (I actually tried scripting the mass-closure at first, with the "gh" > >> utility, but something in the github.com data model was broken, and > >> the server wouldn't allow me to close those PRs with the utility, > >> even though I had authenticated the utility under my account, with a > >> complete set of scopes -- see my report at > >> <https://github.com/cli/cli/discussions/6814>.) > >> > >> Regarding your new PR: it's again good for a personal CI run only. If > >> it completes fine, please post the patches to the mailing list, using > >> git-send-email. > >> > >> (From browsing recent list traffic, it seems that your colleague > >> Moritz Fischer <mori...@google.com> has posted patches like that to > >> the list; please consider consulting with Moritz regarding the git > >> setup (SMTP server etc) withing Google. Cc'ing Moritz now.) > >> > >> When sending the patches like that, please CC the maintainers and > >> reviewers that are supposed to review them. Once they are happy with > >> the series, one of them will submit a PR with them, and set the > >> "push" label on the PR. When the CI run succeeds, the mergify bot > >> will merge *that* PR automatically. > >> > > > > I think we were already way past this with Dionna's work - numerous > > iterations have been posted and discussed, and the merge of the final > > version was only deferred because of the soft freeze. > > > > That may very well be, but the specific PR I closed (3623) was not > queued by a maintainer, nor did it have the "push" label. So I'd expect > that now a maintainer has to submit a new PR, with the patches most > recently approved on the list. > > The official documentation in the wiki at > <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-maintainer-process-for-the-edk-ii-project> > still says the maintainer has to use "git-am" for picking up the > patches, and to submit a new (separate) PR, with the "push" label set. > So I wouldn't expect the "push" label to be set, as an additional > maintainer action, on a contributor-submitted (pre-existent) PR. > > Sorry for obsessing about this, but given the 400+ stale open PRs, it > was literally impossible to tell apart this one (3623) from the rest. > And in retrospect, I do think I was right to close 3623 as well. IIRC, I > left the PRs younger than 2 weeks alone. > > Now, regarding *where* the most recent version of the series lives (i.e. > what needs to be picked up from the list by the maintainer, for > merging), that beats me. We're conducting the present discussion under > the following thread: > > [edk2-devel] [PATCH 0/3] SEV-SNP accepted memory and BeforeExitBootServices > 20221108164616.3251967-1-dionnaglaze@google.com">http://mid.mail-archive.com/20221108164616.3251967-1-dionnaglaze@google.com > https://edk2.groups.io/g/devel/message/96088 > > https://listman.redhat.com/archives/edk2-devel-archive/2022-November/055386.html > > so I'm tempted to think that this is the version that needs to be > merged. > > However, if I check the November 2022 archive, I see v2 as well: > > [edk2-devel] [PATCH v2 0/4] SEV-SNP accepted memory and > BeforeExitBootServices > > https://listman.redhat.com/archives/edk2-devel-archive/2022-November/055402.html > https://edk2.groups.io/g/devel/message/96104 > > Then again, I find, in the v2 thread: > > > https://listman.redhat.com/archives/edk2-devel-archive/2022-December/056363.html > https://edk2.groups.io/g/devel/message/97065 > > where Dionna writes, "we decided to go ahead and consider v1 of this > series as final". > > So, in effect, this patch series had reached v8 previously (in > *October*), as a part of a larger series > (<https://listman.redhat.com/archives/edk2-devel-archive/2022-October/054823.html>), > then got re-started (split-out) as a smaller series, then reached v2 > like that, then v2 got abandoned in favor of v1; all without a feature > tracking BZ linking the *evolution* of the patch series across the > various postings. > > And then we're surprised stuff falls through the cracks, and I remain > the OCD person that alone finds it problematic that the PR tracker and > the BZ tracker are in total shambles ¯\_(ツ)_/¯ > > Anyway, Dionna pinging in this particular thread (= v1 of the split-out > series) is consistent with the comment quoted from v2 qualifying v1 the > final version. As a courtesy, while my temporarily renewed subscription > to this list lasts a bit longer, I'm going to grab the v1 series in full > from the November (gzipped mbox) archive at > <https://listman.redhat.com/archives/edk2-devel-archive/>, and merge it. > > ... I've picked up the patches, picked up the pending feedback tags too, > and compared the result against Dionna's latest PR > <https://github.com/tianocore/edk2/pull/3885>. > > Beyond the Message-Id tags that git-am picked up on my side, I've found > another difference: patch#1 was originally authored by Sophia Wolf > <phiaw...@google.com>, but that fact has been mangled in the first patch > of <https://github.com/tianocore/edk2/pull/3885> -- commit cd4c186f1099. > Namely, in commit cd4c186f1099, the original authorship is recorded as > another line in the commit message, when it should be reflected by the > Author meta-datum in git. So I'm going to stick with what I have, from > the list archive and git-am (git-am correctly translates the From: line > back to the Author meta-datum), for the new PR: > > https://github.com/tianocore/edk2/pull/3889 > > Laszlo >
-- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98380): https://edk2.groups.io/g/devel/message/98380 Mute This Topic: https://groups.io/mt/94894288/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
