From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by mx.groups.io with SMTP id smtpd.web10.59227.1673539756554258865 for ; Thu, 12 Jan 2023 08:09:16 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20210112 header.b=gPjYG1Y3; spf=pass (domain: google.com, ip: 209.85.214.174, mailfrom: dionnaglaze@google.com) Received: by mail-pl1-f174.google.com with SMTP id v23so15780276plo.1 for ; Thu, 12 Jan 2023 08:09:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=aSiN4OR/29IZgxm2Nt/beM1fhSU+CtempAZQdC1O13A=; b=gPjYG1Y3yHPK29UZXQxckuTSuEOfzh/KUTUbVk1SK+JD9Waek8RAJvAIpOCnkgMzQD zx/7VmT5K7hE2oNcx5Qokz6a1TBXC5qrusyHcI5Ly6oL6ExQcZCUEDiAMftrbMArv9gp hg7I7RCFJIQF5sOhU8+HLBUDjHr+lp69/5jbfrfHza2+D9hKKbGKVIz6JNRYXfrJUGwW Wz5Xehv1df9tCgBYY8mLw0TQsUAx1gOJYv+IU/SilJ8w3rCSzwJ0MZKKr5VJN2AZyT7l +6RmFXvhRQJdXIaaHROyYPzWfMgcyqHW8l0/sC/ls/wlo0A/S1wnag3i4aKH7AnToueH c69g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=aSiN4OR/29IZgxm2Nt/beM1fhSU+CtempAZQdC1O13A=; b=LBDJd8tXQWlpSNUgOJ+Mu4ejnhsoDRK715xXh4ZYk6CfMY5DSabnaVnfoEHzywD4eW lAVPPz2ht5cmvkEHHsvEk0vlFz5rhp4EuOGa8WdQbVmxjUK1tGZqlMQ2tvTCVIONM3n5 tbHLhvFD3FWKFJepUiGEjogBAZVY+Qmj/Zm76WvRL6o4rf9q3YeZKoIgkEzX276dUPj2 tTrQYWe8VdYyUhxpjd0YgPYHcj2Gve18kVRTi9+7FMC8dNVWaF4hhm9LIVkpSmxxQBAr 083g0kbGgJiTtGLdw1Vq1l8u6vOQgNHitR9RWnwlIm62hIQW4j5s3kVlyIutDgpDbXKI vvSA== X-Gm-Message-State: AFqh2krFyBFqZOVFn7AAguSKWRphOOldMFo+3SpY6/WtfaO4SKH9VBQX Qpc7kcWJsOtClntZ9mMIOy99VAnNiDsA5OKvnKJv3Q== X-Google-Smtp-Source: AMrXdXsDE0aba8eSbaubm6IdXuRwGowEnpAgCbJCLbsf8zwMQYsrMJx0wDdCFdmkcnV4QYwU7Qrq8YVA/1+bi9SuvQA= X-Received: by 2002:a17:902:d346:b0:194:62be:1a1a with SMTP id l6-20020a170902d34600b0019462be1a1amr149062plk.134.1673539755810; Thu, 12 Jan 2023 08:09:15 -0800 (PST) MIME-Version: 1.0 References: <20221108164616.3251967-1-dionnaglaze@google.com> <20221108164616.3251967-4-dionnaglaze@google.com> <75ed5959-fbb2-3343-e7d7-dfa17b4a2615@redhat.com> <79fc5c62-922c-0db2-2c12-8557d276cce3@redhat.com> In-Reply-To: <79fc5c62-922c-0db2-2c12-8557d276cce3@redhat.com> From: "Dionna Glaze" Date: Thu, 12 Jan 2023 08:09:04 -0800 Message-ID: Subject: Re: [edk2-devel] [PATCH 3/3] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices To: Laszlo Ersek Cc: Ard Biesheuvel , devel@edk2.groups.io, Gerd Hoffmann , James Bottomley , "Yao, Jiewen" , Tom Lendacky , "Xu, Min M" , Andrew Fish , "Ni, Ray" , Moritz Fischer , "Kinney, Michael D" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks for your explanation and fastidiousness, Laszlo. Much appreciated. On Thu, Jan 12, 2023 at 7:32 AM Laszlo Ersek wrote: > > On 1/12/23 14:38, Ard Biesheuvel wrote: > > On Thu, 12 Jan 2023 at 13:24, Laszlo Ersek 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 > >>> wrote: > >>>> > >>>> Hi Dionna, > >>>> > >>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Develop= ment-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 > >> .) > >> > >> 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 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 > > 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 BeforeExitBootServ= ices > 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/05= 5386.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 BeforeExitBootS= ervices > https://listman.redhat.com/archives/edk2-devel-archive/2022-November/05= 5402.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/05= 6363.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 > (), > 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 =C2=AF\_(=E3=83=84)_/=C2=AF > > Anyway, Dionna pinging in this particular thread (=3D 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 > , 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 > . > > 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 > , but that fact has been mangled in the first patch > of -- 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 > --=20 -Dionna Glaze, PhD (she/her)