Message ID | 20231208181802.88528-3-leobras@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp5636004vqy; Fri, 8 Dec 2023 10:19:32 -0800 (PST) X-Google-Smtp-Source: AGHT+IFOkAtKxG3mzhSkAuKbFzcu3bCvlQqYjv7mKXSJdjcPWbV442nqMKP5zPoKl7rgaQZqCDeu X-Received: by 2002:a05:6a20:914f:b0:18b:92b1:37b8 with SMTP id x15-20020a056a20914f00b0018b92b137b8mr499033pzc.33.1702059572117; Fri, 08 Dec 2023 10:19:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702059572; cv=none; d=google.com; s=arc-20160816; b=hH7YFhtYeVGGtaKzcooiicO7dBv+QhhNnQJvNH64bqaU+vRx+NU74+SwtyTvQxSM7g BysGz13aW0eGbkgmyE4gcYPazq8gzjdovCwUrCXX5ugVyyGKwu1eufTlPamNCsflA/sM QTkb2qn+niZjlCUs9WiFPMV4kcGKyk4PYMr8Vw/0VXqQYHD9dQckC9INvdKkLP/tK8/Q z2ZIq4x6YCjqApq5jk8mx1HUJn+wuGnH+wuDnqwtU+BV1Zg0psr+f2Ks2POlyzmRbSzQ oo0oM4CAQo/Incl/gaGBUus2hoGJz1bgemZqGOUF+2jmKTu7iwY6/Bo9N+nwPOLi4C+3 y0+g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=JoC6C0Qt+cZ8XaWFigMJ7rxzgccRqB4PBw4Ey/TxzFU=; fh=F9LrPD1UwuMwYlPSaRErN119oCgL8q6w/VgpD1UeDsc=; b=vX+KwHQ1L7ddmG9nGBfjNyOA1Y+wd1oRxH98DLo+YG7hyDMD6w5mSl2sWMCRnQNSZn zthoKXxF1QbpShc3ziS1VwB9GY69isOAErcXYRTnixTqWa9RJ7RRqIQ07WsVWKKNhCUf 6sGa29lqJ1eujfEZ2FMUn1gHZCMKM7sms1drueoYoVd4z4b/AkIt6buLzJ3dzv/vc9TA n58a9SG3nj82XsPEzP1HxUlK/JxgNBoXBu2iU4ZFRUDOWeu9f5zW5hfcD031Hc9V9Aby B0hx3KB18taX8MsaEHR8bfMqt0KU8uIHQTRbUVqMYf5IESdjmuTpAHf7azukNfU9BIQv x/qg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=cdmIiX2j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id e6-20020a654786000000b005c660ba30a7si1830926pgs.512.2023.12.08.10.19.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 10:19:32 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=cdmIiX2j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id E564881E7816; Fri, 8 Dec 2023 10:19:27 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1574560AbjLHSTT (ORCPT <rfc822;ezelljr.billy@gmail.com> + 99 others); Fri, 8 Dec 2023 13:19:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52516 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1574152AbjLHSTS (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 8 Dec 2023 13:19:18 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9AA3090 for <linux-kernel@vger.kernel.org>; Fri, 8 Dec 2023 10:19:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1702059563; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=JoC6C0Qt+cZ8XaWFigMJ7rxzgccRqB4PBw4Ey/TxzFU=; b=cdmIiX2jP4WG3JyiS7lDzwdYVzUox8P7rSk5m0nfCNEvip4Y3q5/9FeK9kdYE/w2DH5LWK TZh+E5/Y2fJ4DayGcW/lyn4A0XEc4iKZTt5vj6pCkLECPfs1eHGqKFYIZzaX7mrfJE6cdh 7+SuZvuLcul/Q+akRtCYjsRHrku0il4= Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-19-q8stMTSZPQGoqAdbt3OOlw-1; Fri, 08 Dec 2023 13:19:21 -0500 X-MC-Unique: q8stMTSZPQGoqAdbt3OOlw-1 Received: by mail-pj1-f72.google.com with SMTP id 98e67ed59e1d1-286da86884aso2449746a91.2 for <linux-kernel@vger.kernel.org>; Fri, 08 Dec 2023 10:19:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702059560; x=1702664360; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=JoC6C0Qt+cZ8XaWFigMJ7rxzgccRqB4PBw4Ey/TxzFU=; b=fciFHu7x9vLETGU/6gCMBQkt3btORvCQuj5ti8E/mXC52/w+RS1dLggmabd2SIuM0r Qr10YqsbwT6PNrGwU15e2/RMOPcGnzwrCjc6j69MqYCt3tl866FFg34fK9Y/B6Sr2KJ4 IC1UHYTvvxzQddtbI3giS2XIyV3JONZoMlO0rKeTHdlCwRqrwNJWrCFJi60IXzdB+F47 sEE6M2e/BwaQw7yy3D0gWnv9A0y94zKXtVEizydcZ7HNT0vlHUI+uak4RD6mMkQXOqCJ gi8WSPs6Ti3TolQolGnQntLHgjSmXVUyxaHbsLrseUbXmF2KtFORfDay48UrpqrvcYRO c2ow== X-Gm-Message-State: AOJu0YyIHA1Xb3mecQaXtUM16xZkZz93MV5fX17bS1XKdyfDE5sFHq9n YvzVwFemhd0rEnRglM7MGjPgd87spwnveV4O4sq5ptvSwr579/KYk93GNGXyt/8R/dybmqumJms tN0zQHra5zArdLAaFjhlmiuNB X-Received: by 2002:a17:90a:31c1:b0:286:f3ec:2a1 with SMTP id j1-20020a17090a31c100b00286f3ec02a1mr481798pjf.31.1702059560083; Fri, 08 Dec 2023 10:19:20 -0800 (PST) X-Received: by 2002:a17:90a:31c1:b0:286:f3ec:2a1 with SMTP id j1-20020a17090a31c100b00286f3ec02a1mr481786pjf.31.1702059559735; Fri, 08 Dec 2023 10:19:19 -0800 (PST) Received: from localhost.localdomain ([2804:1b3:a802:3102:945e:6f76:fb73:6512]) by smtp.gmail.com with ESMTPSA id nk10-20020a17090b194a00b002865781c51dsm3711169pjb.18.2023.12.08.10.19.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 10:19:19 -0800 (PST) From: Leonardo Bras <leobras@redhat.com> To: Randy Dunlap <rdunlap@infradead.org>, Nicolas Schier <nicolas@fjasle.eu>, Masahiro Yamada <masahiroy@kernel.org>, Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Leonardo Bras <leobras@redhat.com> Cc: linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org Subject: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile Date: Fri, 8 Dec 2023 15:18:03 -0300 Message-ID: <20231208181802.88528-3-leobras@redhat.com> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Fri, 08 Dec 2023 10:19:28 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784467652687479490 X-GMAIL-MSGID: 1784738817596175539 |
Series |
[RFC,v5,1/1] scripts: Introduce a default git.orderFile
|
|
Commit Message
Leonardo Bras Soares Passos
Dec. 8, 2023, 6:18 p.m. UTC
When reviewing patches, it looks much nicer to have some changes shown before others, which allow better understanding of the patch before the the .c files reviewing. Introduce a default git.orderFile, in order to help developers getting the best ordering easier. Signed-off-by: Leonardo Bras <leobras@redhat.com> Acked-by: Randy Dunlap <rdunlap@infradead.org> --- Changes since RFCv4: - Added scripts/* into "build system" section - Added "git-specific" section with this script and .gitignore - Thanks for this feedback Nicolas! Changes since RFCv3: - Added "*types.h" matching so type headers appear before regular headers - Removed line ends ($) in patterns: they previously provided a false-positive - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile in any subdirectory Changes since RFCv2: - Fixed licence comment to from /**/ to # - Fixed filename in how-to comment - Fix build order: Kconfig -> Kbuild -> Makefile - Add *.mk extension - Add line-ends ($) to make sure and get the correct extensions - Thanks Masahiro Yamada for above suggestions! - 1 Ack, thanks Randy! Changes since RFCv1: - Added Kconfig* (thanks Randy Dunlap!) - Changed Kbuild to Kbuild* (improve matching) scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 scripts/git.orderFile
Comments
On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <leobras@redhat.com> wrote: > > When reviewing patches, it looks much nicer to have some changes shown > before others, which allow better understanding of the patch before the > the .c files reviewing. > > Introduce a default git.orderFile, in order to help developers getting the > best ordering easier. > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > Acked-by: Randy Dunlap <rdunlap@infradead.org> > > --- > Changes since RFCv4: > - Added scripts/* into "build system" section > - Added "git-specific" section with this script and .gitignore > - Thanks for this feedback Nicolas! > > Changes since RFCv3: > - Added "*types.h" matching so type headers appear before regular headers > - Removed line ends ($) in patterns: they previously provided a > false-positive > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile > in any subdirectory > > Changes since RFCv2: > - Fixed licence comment to from /**/ to # > - Fixed filename in how-to comment > - Fix build order: Kconfig -> Kbuild -> Makefile > - Add *.mk extension > - Add line-ends ($) to make sure and get the correct extensions > - Thanks Masahiro Yamada for above suggestions! > - 1 Ack, thanks Randy! > > Changes since RFCv1: > - Added Kconfig* (thanks Randy Dunlap!) > - Changed Kbuild to Kbuild* (improve matching) > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > create mode 100644 scripts/git.orderFile > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile > new file mode 100644 > index 0000000000000..31649ff53d22c > --- /dev/null > +++ b/scripts/git.orderFile > @@ -0,0 +1,39 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +# order file for git, to produce patches which are easier to review > +# by diffing the important stuff like header changes first. > +# > +# one-off usage: > +# git diff -O scripts/git.orderFile ... > +# > +# add to git config: > +# git config diff.orderFile scripts/git.orderFile > +# > + > +MAINTAINERS > + > +# Documentation > +Documentation/* > +*.rst > + > +# git-specific > +.gitignore > +scripts/git.orderFile I think scripts/git.orderFile should be part of "scripts/*" below. > + > +# build system > +*Kconfig* > +*Kbuild* > +*Makefile* I do not like this because "foo-Makefile-bar" is not a Makefile, but would match "*Makefile*". If you do not use wildcard at all, 'Makefile' will match to the root-dir and sub-directories. Kconfig */Kconfig* Kbuild Makefile *.mak *.mk scripts/* may satisfy your needs mostly. > +*.mak > +*.mk > +scripts/* > + > +# semantic patches > +*.cocci > + > +# headers > +*types.h > +*.h > + > +# code > +*.c > -- > 2.43.0 >
From: Leonardo Bras <masahiroy@kernel.org> On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote: > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > When reviewing patches, it looks much nicer to have some changes shown > > before others, which allow better understanding of the patch before the > > the .c files reviewing. > > > > Introduce a default git.orderFile, in order to help developers getting the > > best ordering easier. > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > > Acked-by: Randy Dunlap <rdunlap@infradead.org> > > > > --- > > Changes since RFCv4: > > - Added scripts/* into "build system" section > > - Added "git-specific" section with this script and .gitignore > > - Thanks for this feedback Nicolas! > > > > Changes since RFCv3: > > - Added "*types.h" matching so type headers appear before regular headers > > - Removed line ends ($) in patterns: they previously provided a > > false-positive > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile > > in any subdirectory > > > > Changes since RFCv2: > > - Fixed licence comment to from /**/ to # > > - Fixed filename in how-to comment > > - Fix build order: Kconfig -> Kbuild -> Makefile > > - Add *.mk extension > > - Add line-ends ($) to make sure and get the correct extensions > > - Thanks Masahiro Yamada for above suggestions! > > - 1 Ack, thanks Randy! > > > > Changes since RFCv1: > > - Added Kconfig* (thanks Randy Dunlap!) > > - Changed Kbuild to Kbuild* (improve matching) > > > > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 39 insertions(+) > > create mode 100644 scripts/git.orderFile > > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile > > new file mode 100644 > > index 0000000000000..31649ff53d22c > > --- /dev/null > > +++ b/scripts/git.orderFile > > @@ -0,0 +1,39 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > + > > +# order file for git, to produce patches which are easier to review > > +# by diffing the important stuff like header changes first. > > +# > > +# one-off usage: > > +# git diff -O scripts/git.orderFile ... > > +# > > +# add to git config: > > +# git config diff.orderFile scripts/git.orderFile > > +# > > + > > +MAINTAINERS > > + > > +# Documentation > > +Documentation/* > > +*.rst > > + > > +# git-specific > > +.gitignore > > +scripts/git.orderFile > Hello Masahiro, thanks for the feedback! > > > I think scripts/git.orderFile should be part of > "scripts/*" below. > > > > > > > > + > > +# build system > > +*Kconfig* > > +*Kbuild* > > +*Makefile* > > I do not like this because "foo-Makefile-bar" > is not a Makefile, but would match "*Makefile*". That makes sense. > > > If you do not use wildcard at all, 'Makefile' > will match to the root-dir and sub-directories. I tried a quick test here changing an mm/*.c file and mm/Makefile, and the above will print the .c file changes first in any situation here, so it won't have the desired behavior. But if we want to achieve the above we can do so with a slight change in the suggestion: > > > Kconfig > */Kconfig* > Kbuild > Makefile */Makefile > *.mak > *.mk > scripts/* > > > may satisfy your needs mostly. > I have tried the following in the Kernel root: $ find . |grep Makefile |grep -v Makefile$ ./arch/arm/mach-s3c/Makefile.s3c64xx ./arch/mips/Makefile.postlink ./arch/powerpc/Makefile.postlink ./arch/um/Makefile-os-Linux ./arch/um/Makefile-skas ./arch/um/scripts/Makefile.rules ./arch/x86/Makefile_32.cpu ./arch/x86/Makefile.um ./arch/x86/Makefile.postlink ./arch/riscv/Makefile.postlink ./drivers/firmware/efi/libstub/Makefile.zboot ./drivers/usb/serial/Makefile-keyspan_pda_fw [...] $ find . |grep Kbuild |grep -v Kbuild$ ./arch/mips/Kbuild.platforms ./scripts/Kbuild.include Which leads to an honest question: Don't we want to show changes on those files before C files, for example? If so, we need something like: # build system Kconfig* */Kconfig* Kbuild* */Kbuild* Makefile* */Makefile* *.mak *.mk scripts/* It would get rid of "foo-Makefile-bar" case but still match "Makefile-bar" case, which seems to be used around. Is that ok? Thanks! Leo
On Mon, Dec 11, 2023 at 10:14 AM <lsoaresp@redhat.com> wrote: > > From: Leonardo Bras <masahiroy@kernel.org> Sorry about this, there seems to be a bug in my send-email script. I will get it fixed. [...]
On Mon, Dec 11, 2023 at 10:14 PM <lsoaresp@redhat.com> wrote: > > From: Leonardo Bras <masahiroy@kernel.org> > > On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote: > > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > > > When reviewing patches, it looks much nicer to have some changes shown > > > before others, which allow better understanding of the patch before the > > > the .c files reviewing. > > > > > > Introduce a default git.orderFile, in order to help developers getting the > > > best ordering easier. > > > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > > > Acked-by: Randy Dunlap <rdunlap@infradead.org> > > > > > > --- > > > Changes since RFCv4: > > > - Added scripts/* into "build system" section > > > - Added "git-specific" section with this script and .gitignore > > > - Thanks for this feedback Nicolas! > > > > > > Changes since RFCv3: > > > - Added "*types.h" matching so type headers appear before regular headers > > > - Removed line ends ($) in patterns: they previously provided a > > > false-positive > > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile > > > in any subdirectory > > > > > > Changes since RFCv2: > > > - Fixed licence comment to from /**/ to # > > > - Fixed filename in how-to comment > > > - Fix build order: Kconfig -> Kbuild -> Makefile > > > - Add *.mk extension > > > - Add line-ends ($) to make sure and get the correct extensions > > > - Thanks Masahiro Yamada for above suggestions! > > > - 1 Ack, thanks Randy! > > > > > > Changes since RFCv1: > > > - Added Kconfig* (thanks Randy Dunlap!) > > > - Changed Kbuild to Kbuild* (improve matching) > > > > > > > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 39 insertions(+) > > > create mode 100644 scripts/git.orderFile > > > > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile > > > new file mode 100644 > > > index 0000000000000..31649ff53d22c > > > --- /dev/null > > > +++ b/scripts/git.orderFile > > > @@ -0,0 +1,39 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > + > > > +# order file for git, to produce patches which are easier to review > > > +# by diffing the important stuff like header changes first. > > > +# > > > +# one-off usage: > > > +# git diff -O scripts/git.orderFile ... > > > +# > > > +# add to git config: > > > +# git config diff.orderFile scripts/git.orderFile > > > +# > > > + > > > +MAINTAINERS > > > + > > > +# Documentation > > > +Documentation/* > > > +*.rst > > > + > > > +# git-specific > > > +.gitignore > > > +scripts/git.orderFile > > > > Hello Masahiro, thanks for the feedback! > > > > > > > I think scripts/git.orderFile should be part of > > "scripts/*" below. > > > > > > > > > > > > > > > + > > > +# build system > > > +*Kconfig* > > > +*Kbuild* > > > +*Makefile* > > > > I do not like this because "foo-Makefile-bar" > > is not a Makefile, but would match "*Makefile*". > > That makes sense. > > > > > > > If you do not use wildcard at all, 'Makefile' > > will match to the root-dir and sub-directories. > > I tried a quick test here changing an mm/*.c file and mm/Makefile, and the > above will print the .c file changes first in any situation here, so it > won't have the desired behavior. Hmm, you are right. OK, your suggestion below looks good. Thanks. > > But if we want to achieve the above we can do so with a slight change in > the suggestion: > > > > > > > Kconfig > > */Kconfig* > > Kbuild > > Makefile > */Makefile > > *.mak > > *.mk > > scripts/* > > > > > > may satisfy your needs mostly. > > > > I have tried the following in the Kernel root: > > $ find . |grep Makefile |grep -v Makefile$ > ./arch/arm/mach-s3c/Makefile.s3c64xx > ./arch/mips/Makefile.postlink > ./arch/powerpc/Makefile.postlink > ./arch/um/Makefile-os-Linux > ./arch/um/Makefile-skas > ./arch/um/scripts/Makefile.rules > ./arch/x86/Makefile_32.cpu > ./arch/x86/Makefile.um > ./arch/x86/Makefile.postlink > ./arch/riscv/Makefile.postlink > ./drivers/firmware/efi/libstub/Makefile.zboot > ./drivers/usb/serial/Makefile-keyspan_pda_fw > [...] > > $ find . |grep Kbuild |grep -v Kbuild$ > ./arch/mips/Kbuild.platforms > ./scripts/Kbuild.include > > Which leads to an honest question: > Don't we want to show changes on those files before C files, for example? > > If so, we need something like: > > # build system > Kconfig* > */Kconfig* > Kbuild* > */Kbuild* > Makefile* > */Makefile* > *.mak > *.mk > scripts/* > > It would get rid of "foo-Makefile-bar" case but still match > "Makefile-bar" case, which seems to be used around. > > Is that ok? > > Thanks! > Leo > >
From: Leonardo Bras <leobras@redhat.com> On Tue, Dec 12, 2023 at 03:05:38AM +0900, Masahiro Yamada wrote: > On Mon, Dec 11, 2023 at 10:14 PM <lsoaresp@redhat.com> wrote: > > > > From: Leonardo Bras <masahiroy@kernel.org> > > > > On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote: > > > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > > > > > When reviewing patches, it looks much nicer to have some changes shown > > > > before others, which allow better understanding of the patch before the > > > > the .c files reviewing. > > > > > > > > Introduce a default git.orderFile, in order to help developers getting the > > > > best ordering easier. > > > > > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > > > > Acked-by: Randy Dunlap <rdunlap@infradead.org> > > > > > > > > --- > > > > Changes since RFCv4: > > > > - Added scripts/* into "build system" section > > > > - Added "git-specific" section with this script and .gitignore > > > > - Thanks for this feedback Nicolas! > > > > > > > > Changes since RFCv3: > > > > - Added "*types.h" matching so type headers appear before regular headers > > > > - Removed line ends ($) in patterns: they previously provided a > > > > false-positive > > > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile > > > > in any subdirectory > > > > > > > > Changes since RFCv2: > > > > - Fixed licence comment to from /**/ to # > > > > - Fixed filename in how-to comment > > > > - Fix build order: Kconfig -> Kbuild -> Makefile > > > > - Add *.mk extension > > > > - Add line-ends ($) to make sure and get the correct extensions > > > > - Thanks Masahiro Yamada for above suggestions! > > > > - 1 Ack, thanks Randy! > > > > > > > > Changes since RFCv1: > > > > - Added Kconfig* (thanks Randy Dunlap!) > > > > - Changed Kbuild to Kbuild* (improve matching) > > > > > > > > > > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 39 insertions(+) > > > > create mode 100644 scripts/git.orderFile > > > > > > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile > > > > new file mode 100644 > > > > index 0000000000000..31649ff53d22c > > > > --- /dev/null > > > > +++ b/scripts/git.orderFile > > > > @@ -0,0 +1,39 @@ > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > + > > > > +# order file for git, to produce patches which are easier to review > > > > +# by diffing the important stuff like header changes first. > > > > +# > > > > +# one-off usage: > > > > +# git diff -O scripts/git.orderFile ... > > > > +# > > > > +# add to git config: > > > > +# git config diff.orderFile scripts/git.orderFile > > > > +# > > > > + > > > > +MAINTAINERS > > > > + > > > > +# Documentation > > > > +Documentation/* > > > > +*.rst > > > > + > > > > +# git-specific > > > > +.gitignore > > > > +scripts/git.orderFile > > > > > > > Hello Masahiro, thanks for the feedback! > > > > > > > > > > > I think scripts/git.orderFile should be part of > > > "scripts/*" below. > > > > > > > > > > > > > > > > > > > > > > + > > > > +# build system > > > > +*Kconfig* > > > > +*Kbuild* > > > > +*Makefile* > > > > > > I do not like this because "foo-Makefile-bar" > > > is not a Makefile, but would match "*Makefile*". > > > > That makes sense. > > > > > > > > > > > If you do not use wildcard at all, 'Makefile' > > > will match to the root-dir and sub-directories. > > > > I tried a quick test here changing an mm/*.c file and mm/Makefile, and the > > above will print the .c file changes first in any situation here, so it > > won't have the desired behavior. > > > > Hmm, you are right. > > > OK, your suggestion below looks good. > > > Thanks. Thank you for this feedback! I will send a v6 shortly. Thank you! Leo > > > > > > > > > > But if we want to achieve the above we can do so with a slight change in > > the suggestion: > > > > > > >> > > > Kconfig > > > */Kconfig* > > > Kbuild > > > Makefile > > */Makefile > > > *.mak > > > *.mk > > > scripts/* > > > > > > > > > may satisfy your needs mostly. > > > > > > > I have tried the following in the Kernel root: > > > > $ find . |grep Makefile |grep -v Makefile$ > > ./arch/arm/mach-s3c/Makefile.s3c64xx > > ./arch/mips/Makefile.postlink > > ./arch/powerpc/Makefile.postlink > > ./arch/um/Makefile-os-Linux > > ./arch/um/Makefile-skas > > ./arch/um/scripts/Makefile.rules > > ./arch/x86/Makefile_32.cpu > > ./arch/x86/Makefile.um > > ./arch/x86/Makefile.postlink > > ./arch/riscv/Makefile.postlink > > ./drivers/firmware/efi/libstub/Makefile.zboot > > ./drivers/usb/serial/Makefile-keyspan_pda_fw > > [...] > > > > $ find . |grep Kbuild |grep -v Kbuild$ > > ./arch/mips/Kbuild.platforms > > ./scripts/Kbuild.include > > > > Which leads to an honest question: > > Don't we want to show changes on those files before C files, for example? > > > > If so, we need something like: > > > > # build system > > Kconfig* > > */Kconfig* > > Kbuild* > > */Kbuild* > > Makefile* > > */Makefile* > > *.mak > > *.mk > > scripts/* > > > > It would get rid of "foo-Makefile-bar" case but still match > > "Makefile-bar" case, which seems to be used around. > > > > Is that ok? > > > > Thanks! > > Leo > > > > > > > -- > Best Regards > Masahiro Yamada >
From: Leonardo Bras <leobras@redhat.com> On Mon, Dec 11, 2023 at 07:03:50PM -0300, lsoaresp@redhat.com wrote: > From: Leonardo Bras <leobras@redhat.com> > > On Tue, Dec 12, 2023 at 03:05:38AM +0900, Masahiro Yamada wrote: > > On Mon, Dec 11, 2023 at 10:14 PM <lsoaresp@redhat.com> wrote: > > > > > > From: Leonardo Bras <masahiroy@kernel.org> > > > > > > On Sun, Dec 10, 2023 at 04:13:54AM +0900, Masahiro Yamada wrote: > > > > On Sat, Dec 9, 2023 at 3:19 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > > > > > > > When reviewing patches, it looks much nicer to have some changes shown > > > > > before others, which allow better understanding of the patch before the > > > > > the .c files reviewing. > > > > > > > > > > Introduce a default git.orderFile, in order to help developers getting the > > > > > best ordering easier. > > > > > > > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > > > > > Acked-by: Randy Dunlap <rdunlap@infradead.org> > > > > > > > > > > --- > > > > > Changes since RFCv4: > > > > > - Added scripts/* into "build system" section > > > > > - Added "git-specific" section with this script and .gitignore > > > > > - Thanks for this feedback Nicolas! > > > > > > > > > > Changes since RFCv3: > > > > > - Added "*types.h" matching so type headers appear before regular headers > > > > > - Removed line ends ($) in patterns: they previously provided a > > > > > false-positive > > > > > - Fixed build patterns to allow matching Kconfig, Kbuild & Makefile > > > > > in any subdirectory > > > > > > > > > > Changes since RFCv2: > > > > > - Fixed licence comment to from /**/ to # > > > > > - Fixed filename in how-to comment > > > > > - Fix build order: Kconfig -> Kbuild -> Makefile > > > > > - Add *.mk extension > > > > > - Add line-ends ($) to make sure and get the correct extensions > > > > > - Thanks Masahiro Yamada for above suggestions! > > > > > - 1 Ack, thanks Randy! > > > > > > > > > > Changes since RFCv1: > > > > > - Added Kconfig* (thanks Randy Dunlap!) > > > > > - Changed Kbuild to Kbuild* (improve matching) > > > > > > > > > > > > > > > scripts/git.orderFile | 39 +++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 39 insertions(+) > > > > > create mode 100644 scripts/git.orderFile > > > > > > > > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile > > > > > new file mode 100644 > > > > > index 0000000000000..31649ff53d22c > > > > > --- /dev/null > > > > > +++ b/scripts/git.orderFile > > > > > @@ -0,0 +1,39 @@ > > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > > + > > > > > +# order file for git, to produce patches which are easier to review > > > > > +# by diffing the important stuff like header changes first. > > > > > +# > > > > > +# one-off usage: > > > > > +# git diff -O scripts/git.orderFile ... > > > > > +# > > > > > +# add to git config: > > > > > +# git config diff.orderFile scripts/git.orderFile > > > > > +# > > > > > + > > > > > +MAINTAINERS > > > > > + > > > > > +# Documentation > > > > > +Documentation/* > > > > > +*.rst > > > > > + > > > > > +# git-specific > > > > > +.gitignore > > > > > +scripts/git.orderFile > > > > > > > > > > Hello Masahiro, thanks for the feedback! > > > > > > > > > > > > > > > I think scripts/git.orderFile should be part of > > > > "scripts/*" below. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > + > > > > > +# build system > > > > > +*Kconfig* > > > > > +*Kbuild* > > > > > +*Makefile* > > > > > > > > I do not like this because "foo-Makefile-bar" > > > > is not a Makefile, but would match "*Makefile*". > > > > > > That makes sense. > > > > > > > > > > > > > > > If you do not use wildcard at all, 'Makefile' > > > > will match to the root-dir and sub-directories. > > > > > > I tried a quick test here changing an mm/*.c file and mm/Makefile, and the > > > above will print the .c file changes first in any situation here, so it > > > won't have the desired behavior. > > > > > > > > Hmm, you are right. > > > > > > OK, your suggestion below looks good. > > > > > > Thanks. > > Thank you for this feedback! > > I will send a v6 shortly. > > Thank you! > Leo > > > > > > > > > > > > > > > > > > > But if we want to achieve the above we can do so with a slight change in > > > the suggestion: > > > > > > > > > >> > > > > Kconfig > > > > */Kconfig* > > > > Kbuild > > > > Makefile > > > */Makefile > > > > *.mak > > > > *.mk > > > > scripts/* > > > > > > > > > > > > may satisfy your needs mostly. > > > > > > > > > > I have tried the following in the Kernel root: > > > > > > $ find . |grep Makefile |grep -v Makefile$ > > > ./arch/arm/mach-s3c/Makefile.s3c64xx > > > ./arch/mips/Makefile.postlink > > > ./arch/powerpc/Makefile.postlink > > > ./arch/um/Makefile-os-Linux > > > ./arch/um/Makefile-skas > > > ./arch/um/scripts/Makefile.rules > > > ./arch/x86/Makefile_32.cpu > > > ./arch/x86/Makefile.um > > > ./arch/x86/Makefile.postlink > > > ./arch/riscv/Makefile.postlink > > > ./drivers/firmware/efi/libstub/Makefile.zboot > > > ./drivers/usb/serial/Makefile-keyspan_pda_fw > > > [...] > > > > > > $ find . |grep Kbuild |grep -v Kbuild$ > > > ./arch/mips/Kbuild.platforms > > > ./scripts/Kbuild.include > > > > > > Which leads to an honest question: > > > Don't we want to show changes on those files before C files, for example? > > > > > > If so, we need something like: > > > > > > # build system > > > Kconfig* > > > */Kconfig* > > > Kbuild* > > > */Kbuild* > > > Makefile* > > > */Makefile* > > > *.mak > > > *.mk > > > scripts/* > > > > > > It would get rid of "foo-Makefile-bar" case but still match > > > "Makefile-bar" case, which seems to be used around. > > > > > > Is that ok? > > > > > > Thanks! > > > Leo > > > > > > > > > > > > -- > > Best Regards > > Masahiro Yamada > > RFCv6 patch at: https://lore.kernel.org/all/20231211221338.127407-1-leobras@redhat.com/
On Tue, Dec 12, 2023 at 4:32 PM Christoph Hellwig <hch@infradead.org> wrote: > > Urrg, hell no. Alphabetic order is the only one allowing sensible > searching. If you have a different preference use your local .gitconfig > instead of enforcing completely random preference on others. Unlike .gitignore, this feature is opt-in rather than enforced. To use this, you need to run 'git config diff.orderFile scripts/git.orderFile' or 'git diff -C scripts/git.orderFile' Indeed, the file order is subjective, leaving us a question "do we need it in upstream"? At least, it is harmless for people who have no interest. -- Best Regards Masahiro Yamada
On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote: > Unlike .gitignore, this feature is opt-in rather than enforced. > > To use this, you need to run > > 'git config diff.orderFile scripts/git.orderFile' > > or > > 'git diff -C scripts/git.orderFile' Oh, ok. That greatly reduces my concern. > > Indeed, the file order is subjective, leaving > us a question "do we need it in upstream"? > > At least, it is harmless for people who have no interest. .. but this is still a good question. I'm not really sure there is much of a need for it, but as long as it doesn't harm everyone else I'm at least neutral on it.
On Tue, Dec 12, 2023 at 05:08:34AM -0800, Christoph Hellwig wrote: > On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote: > > Unlike .gitignore, this feature is opt-in rather than enforced. > > > > To use this, you need to run > > > > 'git config diff.orderFile scripts/git.orderFile' > > > > or > > > > 'git diff -C scripts/git.orderFile' > > Oh, ok. That greatly reduces my concern. Yes, it's an opt-in, so no user should be directly impacted. > > > > > Indeed, the file order is subjective, leaving > > us a question "do we need it in upstream"? The main idea is patch generation. This file's order is supposed to be the best order for reading a raw patch and understanding the code changes. > > > > At least, it is harmless for people who have no interest. > > .. but this is still a good question. I'm not really sure there is > much of a need for it, but as long as it doesn't harm everyone else > I'm at least neutral on it. diff.orderfile was introduced in git to help order the git diff, and thus the patch generation, in a way that it's easier to understand what the commit / patch intends on doing. Take this example introducing a feature foo, you should see: - Documentation on foo, if introduced - How is foo enabled in build system, if needed - The types / stucts / fields introduced by foo, if any - The interface for using foo, if any - The actual foo implementation. Of course the actual order is open to discussion, and I encourage everyone to suggest any other items or order. Thanks! Leo
On Wed, Dec 13, 2023 at 2:10 AM Leonardo Bras <leobras@redhat.com> wrote: > > On Tue, Dec 12, 2023 at 05:08:34AM -0800, Christoph Hellwig wrote: > > On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote: > > > Unlike .gitignore, this feature is opt-in rather than enforced. > > > > > > To use this, you need to run > > > > > > 'git config diff.orderFile scripts/git.orderFile' > > > > > > or > > > > > > 'git diff -C scripts/git.orderFile' > > > > Oh, ok. That greatly reduces my concern. > > Yes, it's an opt-in, so no user should be directly impacted. Applied to linux-kbuild. Thanks. > > > > > > > > Indeed, the file order is subjective, leaving > > > us a question "do we need it in upstream"? > > The main idea is patch generation. > This file's order is supposed to be the best order for reading a raw patch > and understanding the code changes. > > > > > > > At least, it is harmless for people who have no interest. > > > > .. but this is still a good question. I'm not really sure there is > > much of a need for it, but as long as it doesn't harm everyone else > > I'm at least neutral on it. > > diff.orderfile was introduced in git to help order the git diff, and thus > the patch generation, in a way that it's easier to understand what the > commit / patch intends on doing. > > Take this example introducing a feature foo, you should see: > - Documentation on foo, if introduced > - How is foo enabled in build system, if needed > - The types / stucts / fields introduced by foo, if any > - The interface for using foo, if any > - The actual foo implementation. > > Of course the actual order is open to discussion, and I encourage everyone > to suggest any other items or order. > > Thanks! > Leo > >
On Fri, Dec 15, 2023 at 2:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Dec 13, 2023 at 2:10 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > On Tue, Dec 12, 2023 at 05:08:34AM -0800, Christoph Hellwig wrote: > > > On Tue, Dec 12, 2023 at 05:09:21PM +0900, Masahiro Yamada wrote: > > > > Unlike .gitignore, this feature is opt-in rather than enforced. > > > > > > > > To use this, you need to run > > > > > > > > 'git config diff.orderFile scripts/git.orderFile' > > > > > > > > or > > > > > > > > 'git diff -C scripts/git.orderFile' > > > > > > Oh, ok. That greatly reduces my concern. > > > > Yes, it's an opt-in, so no user should be directly impacted. > > > Applied to linux-kbuild. > Thanks. Thank you! > > > > > > > > > > > > > > > > > Indeed, the file order is subjective, leaving > > > > us a question "do we need it in upstream"? > > > > The main idea is patch generation. > > This file's order is supposed to be the best order for reading a raw patch > > and understanding the code changes. > > > > > > > > > > At least, it is harmless for people who have no interest. > > > > > > .. but this is still a good question. I'm not really sure there is > > > much of a need for it, but as long as it doesn't harm everyone else > > > I'm at least neutral on it. > > > > diff.orderfile was introduced in git to help order the git diff, and thus > > the patch generation, in a way that it's easier to understand what the > > commit / patch intends on doing. > > > > Take this example introducing a feature foo, you should see: > > - Documentation on foo, if introduced > > - How is foo enabled in build system, if needed > > - The types / stucts / fields introduced by foo, if any > > - The interface for using foo, if any > > - The actual foo implementation. > > > > Of course the actual order is open to discussion, and I encourage everyone > > to suggest any other items or order. > > > > Thanks! > > Leo > > > > > > > -- > Best Regards > Masahiro Yamada >
diff --git a/scripts/git.orderFile b/scripts/git.orderFile new file mode 100644 index 0000000000000..31649ff53d22c --- /dev/null +++ b/scripts/git.orderFile @@ -0,0 +1,39 @@ +# SPDX-License-Identifier: GPL-2.0 + +# order file for git, to produce patches which are easier to review +# by diffing the important stuff like header changes first. +# +# one-off usage: +# git diff -O scripts/git.orderFile ... +# +# add to git config: +# git config diff.orderFile scripts/git.orderFile +# + +MAINTAINERS + +# Documentation +Documentation/* +*.rst + +# git-specific +.gitignore +scripts/git.orderFile + +# build system +*Kconfig* +*Kbuild* +*Makefile* +*.mak +*.mk +scripts/* + +# semantic patches +*.cocci + +# headers +*types.h +*.h + +# code +*.c