From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web10.8122.1589877595614222652 for ; Tue, 19 May 2020 01:39:56 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ZYZ48FvN; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589877594; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WdsiSzERuwr4yyRS/sEVTfjg8JGLyx/q+m0/OjPozdM=; b=ZYZ48FvNtkBUHmZiC5hZwOTaFx0MmG9TsD97tizoTfZbAdKAcJRZmTWJvFgbZtPMwM1lYI p9hvILclFWkTMde3dKTTVYVYnxTrXPlqOPw6gLRuoTn9Cld4XJk6PtDu3rSZTrBKtA9MdO DcfUiii9ASvygCulvhdg+XqEMnLK2aM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-381-nMoD-txsNnGDI_KMKuRSSw-1; Tue, 19 May 2020 04:39:52 -0400 X-MC-Unique: nMoD-txsNnGDI_KMKuRSSw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 236E7107ACF3; Tue, 19 May 2020 08:39:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-149.ams2.redhat.com [10.36.114.149]) by smtp.corp.redhat.com (Postfix) with ESMTP id D0B30100239B; Tue, 19 May 2020 08:39:49 +0000 (UTC) Subject: Re: [edk2-devel] [edk2-rfc] GitHub Pull Request based Code Review Process To: "Desimone, Nathaniel L" , "rfc@edk2.groups.io" , "bret.barkelew@microsoft.com" , "devel@edk2.groups.io" , "Kinney, Michael D" References: From: "Laszlo Ersek" Message-ID: Date: Tue, 19 May 2020 10:39:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 05/19/20 09:21, Desimone, Nathaniel L wrote: > However, I would like to register my general endorsement for pull > requests or some other web based system of code review... and I don't > have an Instagram account by the way :) Personally, I prefer Gerrit as > I use it a lot with coreboot and other projects. But since we are > using Github for hosting, pull requests are an easy switch and a > logical choice. My main reason for being excited about pull requests > mostly has to do with the amount of manual effort required to be a > TianoCore maintainer right now. My understanding is that, at this point, we're inevitably going to migrate the contribution/review workflow to GitHub. I believe the switch is going to happen once the email webhook has been deemed functional and stable enough by the community. Digression starts: > Implementing the logic to parse the contents of emails to categorize > them like this required me to define no less than 12 email filter > rules in Microsoft Outlook, and I have to change my filtering logic > every time I am added/removed from a Maintainers.txt file. 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. How you handle messages from then on may be a personal matter of course. I simply tag ("star") such messages (patches / series pending my review), and I revisit my "set of starred messages" every day (sometimes multiple times per day). > I'm sure every other maintainer has spent a time separately > implementing filtering logic like I have. This helps, but still for > every thread, I have to go and check if one of the other maintainers > has already reviewed/pushed that patch series yet, and if not > review/push it. 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). > If I have feedback on a patch series, I have to categorize it as > awaiting response from author and check up on it from time to time, > sometimes I ping the author directly and remind them to send a new > patch series. 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. > Implementing this state machine is a lot of manual work and it kind of > feels like I'm a telephone operator in the 1950s. I greatly welcome > automation here as I am sure it will increase the number of patch > series I am able to review per hour. "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.) Thanks! Laszlo