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 <leobras@xxxxxxxxxx> 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@xxxxxxxxxx> > > > > --- > > > > > > > > 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 > > >