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

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

> 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
> comfort.)

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.

Thanks,
Nate

> Thanks!
> Laszlo
> 
> 
>