Re: [edk2-devel] EDK II Maintainers - EDK II CI is now active on edk2/master

["Michael D Kinney" <michael.d.kinney@intel.com>]

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 <lersek@redhat.com>
> Sent: Wednesday, November 13, 2019 12:57 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Ni, Ray <ray.ni@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