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

Nate DeSimone

On 5/19/20 01:40, Laszlo Ersek wrote:

That seems strange. I have one rule per edk2-* list, for moving such incoming
email into the appropriate list folder. That's all.

While I read all the subject lines (skim all the threads) on edk2-devel,
generally, if you share reviewer or maintainer responsibilities for some
subsystem, then people posting patches for that subsystem are supposed to
CC you explicitly, in addition to messaging the list.
I tend to make the assumption that people do not CC me on the patches that they are supposed to CC me on. So I set up my filtering rules to do a deep inspection of the message contents to see if it touches a package that I maintain.

Checking whether others have commented is near trivial if your MUA
supports a threaded view.

Checking whether a co-maintainer of yours has pushed a given series is also
simple if they diligently report the fact of merging on the list (in the subject
patch threads).
Yes, checking for comments is trivial. However, my fellow co-maintainers are not very diligent on sending push notifications. So when I see comments from one of my fellow co-maintainers I immediately ask myself the question: "Did they already push this, and does it make sense for me to spend time reviewing this patch series?" Answering that question involves a git pull and a review of history in gitk to see what has been done already.

I think this is not your job, as a reviewer/maintainer. Once your review is
complete, or blocked on a question you need an answer to, the ball is back in
the contributor's court. They can answer, or post the next version, whenever
they see fit. Until then, the most they can expect of you is answering any
further questions they might have for understanding your previous feedback
better. You need not push contributors to complete their contributions.
I think my experience is colored somewhat here. I'd say more than half the time, the contributor is another Intel employee. Often times, they are contributing code changes that I asked them to implement. :)

"State machine" is a very good analogy! Personally, I don't find it tiresome.
Yes, it's important to recognize the events (= new emails) that trigger
transitions between states. (For example: when I complete a review, when I
get a new version of a series or a brand new series, when I get asked a
question.) Once I recognize those events correctly, I just diligently massage
said tags ("stars").

And I keep iterating over my set of "starred" messages; I do actual work
(e.g., reviews) in "bottom halves"; detached from new emails.

I don't find this a burden as I have to manage my "real life" with task lists
anyway. Without them, my real life would collapse in a week; so it's nothing
unusual for me. (And no, I don't allow shady cloud-based automatisms to
manage my life for me; I value my privacy way above my
Agreed that I also keep my personal task lists in a paper notebook and manage my real life list manually. However, my real life list is much smaller (since I have most of the context in my head already)... and its private. Everything I do on this mailing list is public anyway, so having some centralized service keep track of state transitions doesn't bother me. The "bottom half" of that state transition is going to generate a public email from my address, so it's not like the current state of the state machine that I'm running in my head is private.



Join to automatically receive all group messages.