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 <[email protected]>
---
Please provide feedback on what else to add / remove / reorder here!
Changes since RFCv1:
- Added Kconfig* (thanks Randy Dunlap!)
- Changed Kbuild to Kbuild* (improve matching)
scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 scripts/git.orderFile
diff --git a/scripts/git.orderFile b/scripts/git.orderFile
new file mode 100644
index 000000000000..819f0a957fe3
--- /dev/null
+++ b/scripts/git.orderFile
@@ -0,0 +1,32 @@
+/* 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
+
+# build system
+Kbuild*
+Kconfig*
+Makefile*
+*.mak
+
+# semantic patches
+*.cocci
+
+# headers
+*.h
+
+# code
+*.c
--
2.42.0
On Wed, Sep 13, 2023 at 04:39:08AM -0300, Leonardo Bras wrote:
> On Tue, Sep 12, 2023 at 04:51:47PM -0300, Leonardo Bras wrote:
> > On Tue, Sep 12, 2023 at 04:53:11PM +0900, Masahiro Yamada wrote:
> > > On Tue, Sep 12, 2023 at 8:45 AM Leonardo Bras <[email protected]> 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 <[email protected]>
> > > > ---
> > > >
> > > > Please provide feedback on what else to add / remove / reorder here!
> > > >
> > > > Changes since RFCv1:
> > > > - Added Kconfig* (thanks Randy Dunlap!)
> > > > - Changed Kbuild to Kbuild* (improve matching)
> > > >
> > > > scripts/git.orderFile | 32 ++++++++++++++++++++++++++++++++
> > > > 1 file changed, 32 insertions(+)
> > > > create mode 100644 scripts/git.orderFile
> > > >
> > > > diff --git a/scripts/git.orderFile b/scripts/git.orderFile
> > > > new file mode 100644
> > > > index 000000000000..819f0a957fe3
> > > > --- /dev/null
> > > > +++ b/scripts/git.orderFile
> > > > @@ -0,0 +1,32 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +
> > >
> > >
> > > Please use "# SPDX-License-Identifier: GPL-2.0".
> > >
> > > /* ... */ is not a valid comment style for the orderfile.
> >
> > Oh, you are right.
> > My bad, it was a last minute change.
> >
> > >
> > >
> > >
> > >
> > > > +# 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
> > >
> > >
> > > These comments are bogus.
> > >
> > >
> > > I guess this comment header was copied from QEMU,
> >
> > Yes, I tried to adapt it from QEMU to kernel needs.
> >
> >
> > > but you changed the file path
> > > from scripts/git.orderfile to scripts/git.orderFile.
> > >
> > >
> > > You need to adjust the comment lines to
> > >
> > >
> > > git diff -O scripts/git.orderFile ...
> > >
> > > git config diff.orderFile scripts/git.orderFile
> > >
> > >
> >
> > Adjusted, thanks!
> >
> > >
> > > Or, you need to get the file path back to scripts/git.orderfile
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > > +#
> > > > +
> > > > +MAINTAINERS
> > > > +
> > > > +# Documentation
> > > > +Documentation/*
> > > > +*.rst
> > > > +
> > > > +# build system
> > > > +Kbuild*
> > > > +Kconfig*
> > > > +Makefile*
> > >
> > >
> > > Kbuild* and Makefile* are interchangeable.
> > > (both are for GNU Make)
> > >
> > > Kconfig* are different types.
> > >
> > >
> > > Better to arrange the order to
> > >
> > > Kconfig*
> > > Kbuild*
> > > Makefile*
> > >
> > >
> >
> > Oh, that makes sense.
> > Done!
> >
> > >
> > >
> > >
> > > > +*.mak
> > >
> > > QEMU consistently uses only *.mak.
> > >
> > > I just realized the kernel tree uses both *.mak and *.mk
> > >
> > > masahiro@zoe:~/ref/linux(master)$ find . -name '*.mak'
> > > ./tools/scripts/utilities.mak
> > > masahiro@zoe:~/ref/linux(master)$ find . -name '*.mk'
> > > ./tools/testing/selftests/lib.mk
> > > ./tools/testing/selftests/ptp/testptp.mk
> >
> > Sure, I will add '*.mk' after '*.mak', getting:
> >
> > # build system
> > Kconfig*
> > Kbuild*
> > Makefile*
> > *.mak
> > *.mk
> >
> >
> > >
> > >
> > >
> > >
> > >
> > > BTW, I quickly tested this, but
> > > it did not work as I expected.
> > >
> > >
> > >
> > >
> > >
> > > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only d34599b^..d34599b
> > > MAINTAINERS
> > > drivers/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Kconfig
> > > drivers/cache/Makefile
> > > drivers/cache/ax45mp_cache.c
> > >
> > > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> > > scripts/git.orderFile d34599b^..d34599b
> > > MAINTAINERS
> > > drivers/cache/ax45mp_cache.c
> > > drivers/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Kconfig
> > > drivers/cache/Makefile
> > >
> > > masahiro@zoe:~/ref/linux(aaa)$ git diff --name-only -O
> > > scripts/git.orderFile d34599b..d34599b^
> > > MAINTAINERS
> > > drivers/cache/ax45mp_cache.c
> > > drivers/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Kconfig
> > > drivers/cache/Makefile
> > >
> > >
> > >
> > >
> > >
> > > My expectation was the following:
> > >
> > > MAINTAINERS
> > > drivers/Kconfig
> > > drivers/cache/Kconfig
> > > drivers/Makefile
> > > drivers/cache/Makefile
> > > drivers/cache/ax45mp_cache.c
> > >
> > >
> > > It did not work like that.
> > > Am I missing something?
> >
> > I can reproduce this same behavior for this commit list, and this is odd.
> >
> > When I added a line-end at the .c extension, it works as expected:
> >
> > *.c$
> >
> > I think this makes sense.
> > Just to make sure, I will add an line-end at every pattern with extension:
> >
> > *.h$
> > *.c$
> > *.mk$
>
> Oh, nevermind. This breaks the matching, and results are crazy.
> I will revert it on a v4.
>
> The real solver is:
> */Kconfig*
> */Kbuild*
> */Makefile*
actually, to match root dir:
*Kconfig*
*Kbuild*
*Makefile*
>
> The thing is that if I add just "Kconfig*" it only matches a Kconfig* in
> the root dir.
>
>
> >
> > and so on.
> > Does that work for you?
> >
> >
> > I will send a v3 soon.
> > Thanks!
> > Leo
> >
> > >
> > >
> > >
> > >
> > >
> > > > +
> > > > +# semantic patches
> > > > +*.cocci
> > > > +
> > > > +# headers
> > > > +*.h
>
> I was talking on a previous thread, and it would probably be interesting
> to add "*types.h" before *.h.
>
> I need to think about a way to filter them out when matching "*.h", or it
> won't work because of:
>
> Git doc:
> "The output order is determined by the order of glob patterns in <orderfile>.
> All files with pathnames that match the first pattern are output first, all
> files with pathnames that match the second pattern (but not the first) are
> output next, and so on."
>
> i.e. the file will be put in the category of the last pattern it matches,
> and it makes harder to get "*types.h" before "*.h".
>
> Trying to think on some solution.
Arg, nevermind. The matching failure caused a lot of confusion in my mind.
It works fine if we do:
*types.h
*.h
*.c
Will send a v4 soon.
>
> > > > +
> > > > +# code
> > > > +*.c
> > > > --
> > > > 2.42.0
> > > >
> > >
> > >
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Masahiro Yamada
> > >