Hi Laszlo, I am also reviewing a number of the PRs.
If a PR is composed of a single commit, and the commit message includes the reference to the Tianocore BZ, then the web view of the PR looks good and includes the link to the BZ. A couple examples: https://github.com/tianocore/edk2/pull/161 https://github.com/tianocore/edk2/pull/162 The PR you referred to below is a patch series that is composed of 2 patches. The individual patches do provide the Tianocore BZ references. When a patch series is submitted for review on the mailing list, we recommend a patch #0 summary that should also contain the Tianocore BZ references. When a PR is opened, an initial comment can be provided. Perhaps the easiest way to make sure the PR is correct is to put the patch #0 Summary contents into this comment when the PR is created for a patch series. This also provides another place the patch #0 summary contents are recorded since those contents are never recorded in the git history. Unfortunately, the specific PR you referenced here did not have complete patch #0 summary contents when it was reviewed on the mailing list. https://edk2.groups.io/g/devel/message/50323 I agree that the Tianocore BZ should be updated with a link to the PR. This is not really an extra step. When a BZ is closed, the BZ should be updated with the range of commits that were pushed. Providing a link to the PR that was merged provides the equivalent information. The SHA hash range of the commits can be provided as well in the same comment that closes the BZ. Thanks, Mike > -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: Wednesday, November 13, 2019 12:57 AM > To: devel@edk2.groups.io; Kinney, Michael D > <michael.d.kin...@intel.com>; Sean Brogan > <sean.bro...@microsoft.com>; Bret Barkelew > <bret.barke...@microsoft.com>; Gao, Liming > <liming....@intel.com>; Ni, Ray <ray...@intel.com> > Subject: Re: [edk2-devel] EDK II Maintainers - EDK II > CI is now active on edk2/master > > Hi again, > > (+Ray) > > On 11/12/19 03:55, Michael D Kinney wrote: > > EDK II Maintainers, > > > > EDK II CI Phase 1 feature is now active on > edk2/master. > > > > Please use a GitHub pull request from a branch in a > personal fork of > > the edk2 repository with a 'push' label to request a > set of patches to > > be pushed to edk2/master. The GitHub PR replaces the > 'git push' > > operation currently used to commit changes to > edk2/master. > > > > You will need to configure your notifications from > the edk2 repository > > to make sure you receive email notifications when the > checks against > > the GitHub PR passes or fails. > > > > If you submit a GitHub Pull Request without the > 'push' > > label, then the CI checks are run and the results are > generated. > > > > Please let us know if there are any questions about > this change in the > > development process. > > now that a few PRs have been merged "in production", > using the above procedure, I'd like to propose an > addition. > > I've received a number of PR notifications, and I > generally have no idea how to associate them with what > happens on the list. My proposal is supposed to improve > on that. > > Note that, I have always suggested / requested > including links, pointing into the mailing list > archive, in Bugzilla tickets for which new versions of > patch sets have been posted. This (brief) proposal is a > natural continuation / extension of that idea. > > - When opening a GitHub PR, as described in the above > procedure, please > *always* include a reference in the PR's title to the > associated TianoCore bugzilla ticket, if there is one. > > - Similarly, please add a link, pointing to the GitHub > PR, to the bugzilla ticket. > > For example, if I look at the following email: > > [tianocore/edk2] Cpu/remove xd (#164) > > it tells me basically *nothing*. And if I open > > https://github.com/tianocore/edk2/pull/164 > > in my web browser, it still tells me nothing. > > The PR title should include a reference to > TianoCore#2329, and > TianoCore#2329 should include a reference to > <https://github.com/tianocore/edk2/pull/164>. > > Yes, this is more work than before. It is necessary > because now we have *more artifacts* related to pushing > (merging) a patch series. Those artifacts should not > just hang in the air. > > Ray: now that the series has been merged, can you > please reference the GitHub PR, and the resultant > commit range, in TianoCore#2329? Also, can you please > close the bugzilla as fixed? > > Thanks, > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#50585): https://edk2.groups.io/g/devel/message/50585 Mute This Topic: https://groups.io/mt/53725670/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
