From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B6E27C7619A for ; Mon, 3 Apr 2023 00:05:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230367AbjDCAF6 convert rfc822-to-8bit (ORCPT ); Sun, 2 Apr 2023 20:05:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54172 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229503AbjDCAF4 (ORCPT ); Sun, 2 Apr 2023 20:05:56 -0400 Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1876E5FC5 for ; Sun, 2 Apr 2023 17:05:55 -0700 (PDT) Received: by mail-qv1-f49.google.com with SMTP id mf5so2247673qvb.4 for ; Sun, 02 Apr 2023 17:05:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680480354; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ICOMbpPN5APoPrdRm4hlFOLpy587/4ZntybStlkTUQk=; b=E/2hDdvMeyJKtdO5MCeYINMYJ3Xx3JVTTvH5auJnaqtTgbF3bP2K7kjrTRD7TL9ZPH 8TahloOsAUHpWXMgHMEmBb6KauzHH+D+I8BTr6RqNqqsp+X47cemVNSGjEoGYLmVTT0k dvIb/24FflZzDeumsQDf9tlbrp4M1q/Hh+5s5hElKueBjxHR+rYE96wbB3vIhIJnlkP+ QP0oEG/z9XJ2d3Pa6O0VathuZbu7IWEtw5ZSqR2r8bm4atipVxoWABVsAj9LinWdJUKb fMFR2l9Xpw57SJPnmd/MKmdCkAPm5iv/kVOiD2rcCnlHcJAalfL1JfBrKScuOQpzwwtW 1qxg== X-Gm-Message-State: AAQBX9dTeeIIYtkouUh3HNuJ9/hldQn5zG2FHFhz80awrTn+8+2ACd8r SQirUcGGCWKfOIGxqxb3YgFYMJkxbKyNVG40gc0= X-Google-Smtp-Source: AKy350bx9+NLJb61rA5eRajxTAItrVCIG6B8VSa4qXBshkSF/vuF5LdPjy/QXwiMBYHNyf++gvfzmvTCcDl0seLIzrY= X-Received: by 2002:ad4:5a12:0:b0:56c:1ff3:9e93 with SMTP id ei18-20020ad45a12000000b0056c1ff39e93mr5943429qvb.5.1680480354118; Sun, 02 Apr 2023 17:05:54 -0700 (PDT) MIME-Version: 1.0 References: <20230103231133.64050-1-robin@jarry.cc> <9a0948e2-d030-ac89-a009-9247054f71f9@gmail.com> In-Reply-To: From: Eric Sunshine Date: Sun, 2 Apr 2023 20:05:43 -0400 Message-ID: Subject: Re: [PATCH] hooks: add sendemail-validate-series To: Robin Jarry Cc: Bagas Sanjaya , git@vger.kernel.org, Junio C Hamano Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Sun, Apr 2, 2023 at 3:19 PM Robin Jarry wrote: > Bagas Sanjaya, Apr 01, 2023 at 04:54: > > In most cases, the patch series is generated by git-format-patch(1). > > When the command is run, it will output: > > > > $ git format-patch -o /tmp --cover-letter --base= > > /tmp/0000-cover-letter.patch > > /tmp/0001-.patch > > /tmp/0002-.patch > > /tmp/0003-.patch > > > > The output can be fed to the hook (as you write). > > > > But I think the hook should also take patch file arguments, for the > > sake of completeness with sendemail-validate hook; that is: > > > > sendemail-validate-series ... > > I don't mind adding this but I am concerned with the maximum size of the > command line arguments when sending large series. Standard input seems > like a safer solution. I share your concern, and don't see a good reason for complicating the implementation _and_ the API by feeding pathnames to the hook as both stdin and as arguments. Feeding them on stdin is the safer choice even if it makes the hook implementation a bit more clunky. > > Also, there should have a check that In-Reply-To must be the first > > patch in the given series or the cover letter (if there is one). > > This is really non-trivial as it depends on the --[no-]chain-reply-to > and --[no-]thread options. Also, the validation occurs before the > message id headers are generated. I'd prefer trusting git-format-patch > to order the patch files properly based on their file names. Moreover, enforcing In-Reply-To: like that would almost certainly be undesirable. It may be policy on _this_ project to chain rerolls like that, but other projects have different policies, even to the point of prohibiting chaining. Such In-Reply-To: enforcement is also well outside the scope of the patch under discussion, as well as being entirely orthogonal. > > Anyway, rather than pinging by random people, I'd like to see [PATCH > > RESEND], rebased on latest git.git tree, ideally with Junio Cc'ed. > > Will do. Thanks. Generally speaking, once you've sent a patch or patch series, when you re-roll, you should _not_ rebase it onto Junio's latest "master" since doing so makes it harder for him to pick up the new version. In this case, your original patch didn't get picked up, so no harm done by rebasing it on "master", but it's good practice to avoid doing so unless there is a strong reason.