* [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile @ 2023-12-08 18:18 Leonardo Bras 2023-12-09 19:13 ` Masahiro Yamada 2023-12-12 7:31 ` Christoph Hellwig 0 siblings, 2 replies; 13+ messages in thread From: Leonardo Bras @ 2023-12-08 18:18 UTC (permalink / raw) To: Randy Dunlap, Nicolas Schier, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, Leonardo Bras Cc: linux-kernel, linux-kbuild 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 + +# build system +*Kconfig* +*Kbuild* +*Makefile* +*.mak +*.mk +scripts/* + +# semantic patches +*.cocci + +# headers +*types.h +*.h + +# code +*.c -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-08 18:18 [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile Leonardo Bras @ 2023-12-09 19:13 ` Masahiro Yamada 2023-12-11 13:13 ` lsoaresp 2023-12-12 7:31 ` Christoph Hellwig 1 sibling, 1 reply; 13+ messages in thread From: Masahiro Yamada @ 2023-12-09 19:13 UTC (permalink / raw) To: Leonardo Bras Cc: Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-09 19:13 ` Masahiro Yamada @ 2023-12-11 13:13 ` lsoaresp 2023-12-11 13:17 ` Leonardo Bras Soares Passos 2023-12-11 18:05 ` Masahiro Yamada 0 siblings, 2 replies; 13+ messages in thread From: lsoaresp @ 2023-12-11 13:13 UTC (permalink / raw) To: Masahiro Yamada Cc: Leonardo Bras, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-11 13:13 ` lsoaresp @ 2023-12-11 13:17 ` Leonardo Bras Soares Passos 2023-12-11 18:05 ` Masahiro Yamada 1 sibling, 0 replies; 13+ messages in thread From: Leonardo Bras Soares Passos @ 2023-12-11 13:17 UTC (permalink / raw) To: Masahiro Yamada Cc: Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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. [...] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-11 13:13 ` lsoaresp 2023-12-11 13:17 ` Leonardo Bras Soares Passos @ 2023-12-11 18:05 ` Masahiro Yamada 2023-12-11 22:03 ` lsoaresp 1 sibling, 1 reply; 13+ messages in thread From: Masahiro Yamada @ 2023-12-11 18:05 UTC (permalink / raw) To: lsoaresp Cc: Leonardo Bras, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 > > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-11 18:05 ` Masahiro Yamada @ 2023-12-11 22:03 ` lsoaresp 2023-12-11 22:41 ` leobras 0 siblings, 1 reply; 13+ messages in thread From: lsoaresp @ 2023-12-11 22:03 UTC (permalink / raw) To: Masahiro Yamada Cc: Leonardo Bras, lsoaresp, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-11 22:03 ` lsoaresp @ 2023-12-11 22:41 ` leobras 0 siblings, 0 replies; 13+ messages in thread From: leobras @ 2023-12-11 22:41 UTC (permalink / raw) To: Masahiro Yamada Cc: Leonardo Bras, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-08 18:18 [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile Leonardo Bras 2023-12-09 19:13 ` Masahiro Yamada @ 2023-12-12 7:31 ` Christoph Hellwig 2023-12-12 8:09 ` Masahiro Yamada 1 sibling, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2023-12-12 7:31 UTC (permalink / raw) To: Leonardo Bras Cc: Randy Dunlap, Nicolas Schier, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-12 7:31 ` Christoph Hellwig @ 2023-12-12 8:09 ` Masahiro Yamada 2023-12-12 13:08 ` Christoph Hellwig 0 siblings, 1 reply; 13+ messages in thread From: Masahiro Yamada @ 2023-12-12 8:09 UTC (permalink / raw) To: Christoph Hellwig Cc: Leonardo Bras, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-12 8:09 ` Masahiro Yamada @ 2023-12-12 13:08 ` Christoph Hellwig 2023-12-12 17:09 ` Leonardo Bras 0 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2023-12-12 13:08 UTC (permalink / raw) To: Masahiro Yamada Cc: Christoph Hellwig, Leonardo Bras, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-12 13:08 ` Christoph Hellwig @ 2023-12-12 17:09 ` Leonardo Bras 2023-12-15 17:02 ` Masahiro Yamada 0 siblings, 1 reply; 13+ messages in thread From: Leonardo Bras @ 2023-12-12 17:09 UTC (permalink / raw) To: Christoph Hellwig Cc: Leonardo Bras, Masahiro Yamada, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-12 17:09 ` Leonardo Bras @ 2023-12-15 17:02 ` Masahiro Yamada 2023-12-15 18:30 ` Leonardo Bras Soares Passos 0 siblings, 1 reply; 13+ messages in thread From: Masahiro Yamada @ 2023-12-15 17:02 UTC (permalink / raw) To: Leonardo Bras Cc: Christoph Hellwig, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 > > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile 2023-12-15 17:02 ` Masahiro Yamada @ 2023-12-15 18:30 ` Leonardo Bras Soares Passos 0 siblings, 0 replies; 13+ messages in thread From: Leonardo Bras Soares Passos @ 2023-12-15 18:30 UTC (permalink / raw) To: Masahiro Yamada Cc: Christoph Hellwig, Randy Dunlap, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Mauro Carvalho Chehab, linux-kernel, linux-kbuild 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-12-15 18:30 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-08 18:18 [RFC PATCH v5 1/1] scripts: Introduce a default git.orderFile Leonardo Bras 2023-12-09 19:13 ` Masahiro Yamada 2023-12-11 13:13 ` lsoaresp 2023-12-11 13:17 ` Leonardo Bras Soares Passos 2023-12-11 18:05 ` Masahiro Yamada 2023-12-11 22:03 ` lsoaresp 2023-12-11 22:41 ` leobras 2023-12-12 7:31 ` Christoph Hellwig 2023-12-12 8:09 ` Masahiro Yamada 2023-12-12 13:08 ` Christoph Hellwig 2023-12-12 17:09 ` Leonardo Bras 2023-12-15 17:02 ` Masahiro Yamada 2023-12-15 18:30 ` Leonardo Bras Soares Passos
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox