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

["Laszlo Ersek" <lersek@redhat.com>]

On 11/27/19 20:03, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> Do you want to see the presence of the 'push' label
> in a notification when the PR is created, or just
> when the checks have been completed in either the
> pass or the fail state?

I'd like to see the purpose of the PR in the first notification email
that is sent out about the PR. Whoever submits the PR clearly knows what
they want with the PR, so they should communicate it. Placing some kind
of token in the subject of the email(s) would be great (very visible).

> A PR may be created initially without the 'push' label.

Yes, I noticed that when I first experimented with the feature. I found
it a bit surprising (but not a problem).

> The EDK II Maintainer can resolve any issues in that PR
> using forced pushes to restart the checks with the 
> issues resolved.

Yes, this is valid for a personal CI build. (Keep trying until it passes.)

> When all check pass, the maintainer can
> then set the 'push' label and re-open the PR.  Then the
> checks will run again and the PR will be merged.

This -- that is, trying to push until it succeeds -- is an invalid
pattern for merging patches into edk2 master, I believe.

I seem to recall an agreement on the list that, in case of conflicts,
the revised patch set -- with the conflicts resolved -- should be
reposted to the list for review. Or else, minimally, the maintainer
should report back with an explanation, or even the actual diff, of the
(minimal!) changes the maintainer had to incorporate, to resolve the
conflicts.

I believe we discussed this in connection to whether we'd allow
github.com to auto-resolve conflicts. We said we wouldn't; every
conflict would need human inspection. And if changes were necessary,
relative to the reviewed patch series, those should

- either be minimal; implemented, and also reported back to the list, by
the maintainer,
- or more extensive, and reposted by the original contributor.

I'd like as few as possible code changes to happen outside of the normal
review process. (Note: I'm not saying "outside of the mailing list" --
once github.com satisfies all the requirements, our normal review
process may become github.com based. And then, on that platform,
significant conflict resolution should also not occur without re-reviewing.)

> In this use case what email notifications would you like to see?

So this question doesn't seem to apply. It's OK if the maintainer does
not add the "push" label immediately (given the UI, it's not even
possible to do so). But a PR should be forever associated with a single
purpose at "birth" (personal CI build, or actual push attempt), and the
notification emails should reflect that purpose, from the start.

(IOW, I don't think it makes sense to "upgrade" a PR from "personal CI
build" to "push to master".)

Thanks!
Laszlo