From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.19568.1679060677312692595 for ; Fri, 17 Mar 2023 06:44:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QVCUfcKJ; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1679060676; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Tbwq6XBuZdZB2PR3sbfVcGz83woD7k54pzeXUtGG0QU=; b=QVCUfcKJLtD8BzCN51HNKy+CwY6MRcpbihfIpi5uizon4Y4hYZY2C6z6iRJB3yw1+vxc42 XdIGEv45ZtjEa/EZ/6JU98HjwJmbBD+8bRrebu+TBExLCfx5/8N8hkrBIRD8vJkijRtVmy A0cr+gGZwvoSTiNoz6fA/HNmO2bOfaY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-246-ieg1CnXvN-ubwr-fLFcWiA-1; Fri, 17 Mar 2023 09:44:35 -0400 X-MC-Unique: ieg1CnXvN-ubwr-fLFcWiA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C772985A5B1; Fri, 17 Mar 2023 13:44:34 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.89]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 768EF40282F; Fri, 17 Mar 2023 13:44:34 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 384AD1800617; Fri, 17 Mar 2023 14:44:33 +0100 (CET) Date: Fri, 17 Mar 2023 14:44:33 +0100 From: "Gerd Hoffmann" To: Marvin =?utf-8?Q?H=C3=A4user?= Cc: Rebecca Cran , edk2-devel-groups-io , Michael Kinney Subject: Re: [edk2-devel] [edk2-platforms] Enable GitHub PR, protected branches, and 'push' label Message-ID: <20230317134433.nl5auf5u55jvfw4t@sirius.home.kraxel.org> References: <7D4DC093-E77D-469C-A1C1-E2BB6B12DE0C@posteo.de> MIME-Version: 1.0 In-Reply-To: <7D4DC093-E77D-469C-A1C1-E2BB6B12DE0C@posteo.de> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Fri, Mar 17, 2023 at 12:32:15PM +0000, Marvin Häuser wrote: > Hi Rebecca and Gerd, > > Replying to 2 mails at once... > > > On 17. Mar 2023, at 11:36, Rebecca Cran wrote: > > > > I like that proposed workflow. > > > > I've also been wondering if we could consider choosing a different > > product for patch reviews that supports our desired workflow better, > > such as Gitlab or Phorge (the new Phabricator project). > > I'd be very cautious with suggesting / approving more tooling. It gets > more confusing (what is hosted where), it gets more complicated to > maintain (who hosts what and is "guaranteed" to be available to fix > things), and so on. Agree. Also from the web-based review tools I've worked with so far (not much, only github and gitlab) github is the better one. > >> (1) In my experience reviewing patches, especially more complex ones, > >> works better in email than in github PR workflows. > > I have no experience with things like large-scale patch set review in, > say, projects like the Linux kernel. However, in about 7 years of > watching edk2-devel and opportunistically participating in patch > review myself, I never once encountered something about mail patch > review that made me think "oh, that's neat". Quite the opposite - I > cannot easily cross-reference when commenting, I cannot easily see > more context to the changed lines, and I cannot easily see the end > result after all patches in a series have been applied. These are all > things that GitHub allows me to do. I keep hearing mail patches "work > better", but I never found convincing reasons for these claims. Mind > sharing? :) (1) Navigation works better for me. On the email side I have the freedom to pick whatever client I like and can configure it the way I like. (2) I can easily automate things. For example it's just two key strokes in the mail client to run a script which creates a new branch and applies the whole patch series. The latter is what I usually do when I want compile and test the series, or when I need something plain email doesn't give me (like getting more patch context, which indeed is a nice github feature). > >> (2) github doesn't preserve stuff like a mail archive does. When a > >> patch series goes through multiple revision github only preserves > >> the latest revision which was actually merged. > (I’m not sure whether the old stuff isn’t eventually wiped, though, > maybe worth carefully inspecting the documentation for options). Yes, this. For active PRs this usually isn't much of a problem. But try come back after a few months, or even a few years (see Rebecca trying to lookup context for a 2016 commit in the archives). > >> * developer opens a draft PR to run CI for the patch series. > >> * when the series passes CI and is ready un-draft the PR. > >> * github action sends the patch series to the edk2-devel list > >> for review (maybe only after CI passed ...). > >> * patch review happens on the list. > >> * in case the developer pushes updates to the branch in response to > >> review comments the github action posts v2/v3 of the series too. > >> * once review is done merge the PR. > > That would at least be a lot better than what we have now. While discussing tooling: Can we move from bugzilla to github issues for bug tracking? That will give us some nice automation and integration benefits. As far I know the blocker for doing that was github issues not having a permission system, which is bad for reporting security bugs. But with security bug reporting and processing using github security advisories now this point should be moot, no? The big problem here is what to do with bugzilla. Migrate all bugs over? Not sure whenever any tooling exists for that already. I suspect we would not be the first ones trying to do that. Or switch bugzilla into readonly mode and keep it running that way for archive purposes? Would have the advantage that all the bugzilla links in the commit messages continue working. take care, Gerd