[PATCH kvm-unit-tests] Add git.orderFile

[PATCH kvm-unit-tests] Add git.orderFile

[Andrew Jones]

It's helpful to have a standard sequence of file types in patches.
Applying git-diff's orderFile feature allows us to do that. Just
do 'git config diff.orderFile scripts/git.orderfile' to enable it.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
This was inspired by Laszlo's patch for QEMU. Anyway it's high time
somebody posts some good kvm-unit-tests bikeshedding bait!
---
 README                |  4 ++++
 scripts/git.orderfile | 15 +++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 scripts/git.orderfile

diff --git a/README b/README
index f8f196d90bf1..73fdd526aed7 100644
--- a/README
+++ b/README
@@ -66,3 +66,7 @@ You can add the following to .git/config to do this automatically for you:
 [format]
 	subjectprefix = kvm-unit-tests PATCH
 
+Additionally it's helpful to have a standard sequence of file types in
+patches. Applying git-diff's orderFile feature allows us to do that.
+The orderFile we use is scripts/git.orderfile. Adding the diff.orderFile
+config (git config diff.orderFile scripts/git.orderfile) will enable it.
diff --git a/scripts/git.orderfile b/scripts/git.orderfile
new file mode 100644
index 000000000000..b98a0e78a37e
--- /dev/null
+++ b/scripts/git.orderfile
@@ -0,0 +1,15 @@
+COPYRIGHT
+MAINTAINERS
+*README
+configure
+*Makefile*
+*.mak
+*.py
+*.bash
+*/run
+*.sh
+*.cfg
+*.lds
+*.h
+*.S
+*.c
-- 
2.9.3

Re: [PATCH kvm-unit-tests] Add git.orderFile

[Radim Krčmář]

2016-12-08 16:39+0100, Andrew Jones:
> It's helpful to have a standard sequence of file types in patches.
> Applying git-diff's orderFile feature allows us to do that. Just
> do 'git config diff.orderFile scripts/git.orderfile' to enable it.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> This was inspired by Laszlo's patch for QEMU. Anyway it's high time
> somebody posts some good kvm-unit-tests bikeshedding bait!

I raise 10 hours of our lives!

> ---
>  README                |  4 ++++
>  scripts/git.orderfile | 15 +++++++++++++++
>  2 files changed, 19 insertions(+)
>  create mode 100644 scripts/git.orderfile
> 
> diff --git a/README b/README
> index f8f196d90bf1..73fdd526aed7 100644
> --- a/README
> +++ b/README
> @@ -66,3 +66,7 @@ You can add the following to .git/config to do this automatically for you:
>  [format]
>  	subjectprefix = kvm-unit-tests PATCH
>  
> +Additionally it's helpful to have a standard sequence of file types in
> +patches. Applying git-diff's orderFile feature allows us to do that.

We already have a standard sequence -- alphabetic order. ;)

I assume the goal is to move the abstract parts to the top of the patch
and minute detail to the bottom, allowing reviewers to understand few
more things on the first read.

> +The orderFile we use is scripts/git.orderfile. Adding the diff.orderFile
> +config (git config diff.orderFile scripts/git.orderfile) will enable it.

The only thing we don't want to be changing afterwards is the file path.

What about scripts/git.order?
The file suffix is pretty obvious and other similar git files don't use
file suffix.  e.g. core.excludeFile -> .gitignore and
core.attributesFile -> .gitattributes.

> diff --git a/scripts/git.orderfile b/scripts/git.orderfile
> new file mode 100644
> index 000000000000..b98a0e78a37e
> --- /dev/null
> +++ b/scripts/git.orderfile
> @@ -0,0 +1,15 @@
> +COPYRIGHT
> +MAINTAINERS
> +*README
> +configure
> +*Makefile*
> +*.mak
> +*.py
> +*.bash
> +*/run
> +*.sh
> +*.cfg
> +*.lds
> +*.h
> +*.S
> +*.c

Let's treat lib/ specially, so the lib/ <-> $arch/ order of hunks
remains the same on all arches.

Whew, this bikeshedding stuff is good ... I plan to apply on Monday, to
let other people have some fun as well.

Thanks.

Re: [PATCH kvm-unit-tests] Add git.orderFile

[Andrew Jones]

On Thu, Dec 08, 2016 at 07:18:03PM +0100, Radim Krčmář wrote:
> 2016-12-08 16:39+0100, Andrew Jones:
> > It's helpful to have a standard sequence of file types in patches.
> > Applying git-diff's orderFile feature allows us to do that. Just
> > do 'git config diff.orderFile scripts/git.orderfile' to enable it.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> > This was inspired by Laszlo's patch for QEMU. Anyway it's high time
> > somebody posts some good kvm-unit-tests bikeshedding bait!
> 
> I raise 10 hours of our lives!
> 
> > ---
> >  README                |  4 ++++
> >  scripts/git.orderfile | 15 +++++++++++++++
> >  2 files changed, 19 insertions(+)
> >  create mode 100644 scripts/git.orderfile
> > 
> > diff --git a/README b/README
> > index f8f196d90bf1..73fdd526aed7 100644
> > --- a/README
> > +++ b/README
> > @@ -66,3 +66,7 @@ You can add the following to .git/config to do this automatically for you:
> >  [format]
> >  	subjectprefix = kvm-unit-tests PATCH
> >  
> > +Additionally it's helpful to have a standard sequence of file types in
> > +patches. Applying git-diff's orderFile feature allows us to do that.
> 
> We already have a standard sequence -- alphabetic order. ;)

Well, unless people use their own random orderFiles, but yeah, I should
have worded that differently.

> 
> I assume the goal is to move the abstract parts to the top of the patch
> and minute detail to the bottom, allowing reviewers to understand few
> more things on the first read.

Right.  I should have stolen more than the idea from Laszlo, I should
have also stolen some of his text too "this list of patterns will place
the more declarative / abstract hunks first, while changes to imperative
code / details will be near the end of the patches. This saves on
scrolling / searching and makes for easier reviewing."

> 
> > +The orderFile we use is scripts/git.orderfile. Adding the diff.orderFile
> > +config (git config diff.orderFile scripts/git.orderfile) will enable it.
> 
> The only thing we don't want to be changing afterwards is the file path.
> 
> What about scripts/git.order?
> The file suffix is pretty obvious and other similar git files don't use
> file suffix.  e.g. core.excludeFile -> .gitignore and
> core.attributesFile -> .gitattributes.

Works for me, however I did steal the filename from Laszlo's QEMU patch,
so, FWIW, scripts/git.orderfile is consistent with QEMU. If we don't
want the QEMU name, then I propose git.difforder.

> 
> > diff --git a/scripts/git.orderfile b/scripts/git.orderfile
> > new file mode 100644
> > index 000000000000..b98a0e78a37e
> > --- /dev/null
> > +++ b/scripts/git.orderfile
> > @@ -0,0 +1,15 @@
> > +COPYRIGHT
> > +MAINTAINERS
> > +*README
> > +configure
> > +*Makefile*
> > +*.mak
> > +*.py
> > +*.bash
> > +*/run
> > +*.sh
> > +*.cfg
> > +*.lds
> > +*.h
> > +*.S
> > +*.c
> 
> Let's treat lib/ specially, so the lib/ <-> $arch/ order of hunks
> remains the same on all arches.

That's a good idea. I should have put lib/ above $arch/ to ensure we
see the common code additions before their use when reviewing.

> 
> Whew, this bikeshedding stuff is good ... I plan to apply on Monday, to
> let other people have some fun as well.

I hope you saved some comments for v2!

Thanks,
drew

> 
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html