[RFC,v5,1/1] scripts: Introduce a default git.orderFile

Message ID 20231208181802.88528-3-leobras@redhat.com
State New
Headers
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

Masahiro Yamada Dec. 9, 2023, 7:13 p.m. UTC | #1
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
>
  
lsoaresp@redhat.com Dec. 11, 2023, 1:13 p.m. UTC | #2
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
  
Leonardo Bras Soares Passos Dec. 11, 2023, 1:17 p.m. UTC | #3
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.

[...]
  
Masahiro Yamada Dec. 11, 2023, 6:05 p.m. UTC | #4
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
>
>
  
lsoaresp@redhat.com Dec. 11, 2023, 10:03 p.m. UTC | #5
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
>
  
Leonardo Bras Soares Passos Dec. 11, 2023, 10:41 p.m. UTC | #6
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/
  
Masahiro Yamada Dec. 12, 2023, 8:09 a.m. UTC | #7
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
  
Christoph Hellwig Dec. 12, 2023, 1:08 p.m. UTC | #8
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.
  
Leonardo Bras Soares Passos Dec. 12, 2023, 5:09 p.m. UTC | #9
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
  
Masahiro Yamada Dec. 15, 2023, 5:02 p.m. UTC | #10
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
>
>
  
Leonardo Bras Soares Passos Dec. 15, 2023, 6:30 p.m. UTC | #11
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
>
  

Patch

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