Re: [EXTERNAL] Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process


Laszlo Ersek
 

On 05/22/20 07:48, Bret Barkelew wrote:

In Mu we have a similar problem of keeping track of what features/bugs
have already been upstreamed and when can they be dropped during an
upstream integration, so that's the more personal interest I have in
such automation.
Proposal:

- Whenever upstreaming a bugfix or a feature, open an upstream BZ.

- In your downstream ticket for the same bugfix or feature,
cross-reference the upstream BZ URL. This shouldn't be a normal comment,
but a dedicated field. In Bugzilla, there is "See Also" (it can carry a
list of URLs). In our own (RH) Bugzilla instance, "See Also" has been
replaced with an "External Trackers" list, but the idea is the same.

- When you rebase, run a git-log over the upstream commit history being
straddled, and collect the upstream BZs referenced. For example:

$ git log edk2-stable201911..edk2-stable202002 \
| grep -E -o 'https://bugzilla.tianocore.org/show_bug.cgi\?id=[0-9]+' \
| sort -u

This reliably presents the set of upstream BZs that were *touched on* in
the subject development cycle, because TianoCore contributors diligently
reference BZs in commit messages. Right? :)

- Use a script to fetch the fresh status of each of those BZ URLs,
because in some cases, "touched on a BZ" does not guarantee "fixed BZ".
Some BZs may require multiple waves of patches.

Of course, BZs that *have* been fixed will all report RESOLVED|FIXED,
because TianoCore contributors and maintainers diligently close BZs as
FIXED when the corresponding patches are merged. They even mention the
commit range(s) implementing the related code changes, without fail.
Right? :)

- Once you have your set of Really Fixed (TM) upstream BZs, run a search
in your downstream tracker to locate the referring downstream tickets,
checking the "See Also" (etc) fields.


In a more serious tone: while Red Hat preaches and practices "upstream
first", we obviously *do* have downstream tickets for bugfixes and
features. And if we are *inheriting* patches for them via a rebase (as
opposed to backporting / cherry-picking them), then we benefit from the
same kind of linkage. That's why I keep "lecturing" maintainers when
they fail to close BZs, and/or to note the subject commit ranges (which
I might want to investigate manually).

Now, I realize that "git forges" can auto-close tickets when
encountering ticket references in merged patches. The problem is that
*multiple* patches may reference a ticket and *still* not constitute a
complete fix for that ticket -- see my "multiple waves of patches" note
above. Automation cannot fully supplant manual ticket wrangling.


NB, the above procedure could also help with composing the "feature
list" for any upcoming edk2 stable tag. When collecting the URLs, and
checking their fresh statuses, also check the "Product" fields. If
Product is "TianoCore Feature Requests", then the ticket is a good
candidate to name at
<https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning#proposed-features>.

Thanks,
Laszlo

Join rfc@edk2.groups.io to automatically receive all group messages.