diff mbox

scripts: add "git.orderfile" for ordering diff hunks by pathname patterns

Message ID 20161202104037.5456-1-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Dec. 2, 2016, 10:40 a.m. UTC 
When passed to git-diff (and to every other git command producing diffs
and/or diffstats) with "-O" or "diff.orderFile", 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.

We intend to advise contributors in the Wiki to run

  git config diff.orderFile scripts/git.orderfile

once, as part of their initial setup, before formatting their first (or,
for repeat contributors, next) patches.

See the "-O" option and the "diff.orderFile" configuration variable in
git-diff(1) and git-config(1).

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Fam Zheng <famz@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

---

Notes:
    I think I managed to incorporate everyone's feedback!

 scripts/git.orderfile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 scripts/git.orderfile

-- 
2.9.2

Comments

Gerd Hoffmann Dec. 2, 2016, 10:59 a.m. UTC | #1 
>   git config diff.orderFile scripts/git.orderfile


> diff --git a/scripts/git.orderfile b/scripts/git.orderfile

> new file mode 100644

> index 000000000000..600a2e4fc540

> --- /dev/null

> +++ b/scripts/git.orderfile

> @@ -0,0 +1,15 @@


Is it possible to have comments in here?  So we can place the git config
command above into into this file too?

> +*.txt

> +configure

> +GNUmakefile

> +makefile

> +Makefile


We also have Makefile.objs and Makefile.target, so "Makefile*" (like the
first version had IIRC) should work better.

> +*.mak

> +qapi-schema*.json

> +qapi/*.json

> +include/qapi/visitor.h

> +include/qapi/visitor-impl.h

> +scripts/qapi.py

> +scripts/*.py

> +*.h

> +qapi/qapi-visit-core.c


I guess there is more which could be placed here (i.e. files holding
infrastructure which makes sense to see first).  But we can easily
refine that incrementally.  That is the point to have this file in the
repo in the first place ;)

cheers,
  Gerd
Laszlo Ersek Dec. 2, 2016, 12:11 p.m. UTC | #2 
On 12/02/16 11:59, Gerd Hoffmann wrote:
>>   git config diff.orderFile scripts/git.orderfile

> 

>> diff --git a/scripts/git.orderfile b/scripts/git.orderfile

>> new file mode 100644

>> index 000000000000..600a2e4fc540

>> --- /dev/null

>> +++ b/scripts/git.orderfile

>> @@ -0,0 +1,15 @@

> 

> Is it possible to have comments in here?  So we can place the git config

> command above into into this file too?


The documentation is silent on this, and I didn't want to experiment, so
I checked the git source code. Yes, comments seem to be supported, see
the prepare_order() function in "diffcore-order.c":

			/* cp to ep has one line */
			if (*cp == '\n' || *cp == '#')
				; /* comment */

How about the following comment:

# Apply this diff order to your git configuration with the command
#
#   git config diff.orderFile scripts/git.orderfile

?

Thanks
Laszlo

>> +*.txt

>> +configure

>> +GNUmakefile

>> +makefile

>> +Makefile

> 

> We also have Makefile.objs and Makefile.target, so "Makefile*" (like the

> first version had IIRC) should work better.

> 

>> +*.mak

>> +qapi-schema*.json

>> +qapi/*.json

>> +include/qapi/visitor.h

>> +include/qapi/visitor-impl.h

>> +scripts/qapi.py

>> +scripts/*.py

>> +*.h

>> +qapi/qapi-visit-core.c

> 

> I guess there is more which could be placed here (i.e. files holding

> infrastructure which makes sense to see first).  But we can easily

> refine that incrementally.  That is the point to have this file in the

> repo in the first place ;)

> 

> cheers,

>   Gerd

> 

>
Gerd Hoffmann Dec. 2, 2016, 12:32 p.m. UTC | #3 
> # Apply this diff order to your git configuration with the command

> #

> #   git config diff.orderFile scripts/git.orderfile


Looks good to me.

cheers,
  Gerd
Max Reitz Dec. 2, 2016, 2:33 p.m. UTC | #4 
On 02.12.2016 11:40, Laszlo Ersek wrote:
> When passed to git-diff (and to every other git command producing diffs

> and/or diffstats) with "-O" or "diff.orderFile", 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.

> 

> We intend to advise contributors in the Wiki to run

> 

>   git config diff.orderFile scripts/git.orderfile

> 

> once, as part of their initial setup, before formatting their first (or,

> for repeat contributors, next) patches.

> 

> See the "-O" option and the "diff.orderFile" configuration variable in

> git-diff(1) and git-config(1).

> 

> Cc: "Michael S. Tsirkin" <mst@redhat.com>

> Cc: Eric Blake <eblake@redhat.com>

> Cc: Fam Zheng <famz@redhat.com>

> Cc: Gerd Hoffmann <kraxel@redhat.com>

> Cc: John Snow <jsnow@redhat.com>

> Cc: Stefan Hajnoczi <stefanha@gmail.com>

> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

> ---

> 

> Notes:

>     I think I managed to incorporate everyone's feedback!

> 

>  scripts/git.orderfile | 15 +++++++++++++++

>  1 file changed, 15 insertions(+)

>  create mode 100644 scripts/git.orderfile

> 

> diff --git a/scripts/git.orderfile b/scripts/git.orderfile

> new file mode 100644

> index 000000000000..600a2e4fc540

> --- /dev/null

> +++ b/scripts/git.orderfile

> @@ -0,0 +1,15 @@

> +*.txt


We also have *.md. (OK, OK, we have a single docs/bitmaps.md, but maybe
it's going to be more in the future. :-))

Max

> +configure

> +GNUmakefile

> +makefile

> +Makefile

> +*.mak

> +qapi-schema*.json

> +qapi/*.json

> +include/qapi/visitor.h

> +include/qapi/visitor-impl.h

> +scripts/qapi.py

> +scripts/*.py

> +*.h

> +qapi/qapi-visit-core.c

> +*.c

>
Fam Zheng Dec. 2, 2016, 2:41 p.m. UTC | #5 
On Fri, 12/02 15:33, Max Reitz wrote:
> On 02.12.2016 11:40, Laszlo Ersek wrote:

> > When passed to git-diff (and to every other git command producing diffs

> > and/or diffstats) with "-O" or "diff.orderFile", 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.

> > 

> > We intend to advise contributors in the Wiki to run

> > 

> >   git config diff.orderFile scripts/git.orderfile

> > 

> > once, as part of their initial setup, before formatting their first (or,

> > for repeat contributors, next) patches.

> > 

> > See the "-O" option and the "diff.orderFile" configuration variable in

> > git-diff(1) and git-config(1).

> > 

> > Cc: "Michael S. Tsirkin" <mst@redhat.com>

> > Cc: Eric Blake <eblake@redhat.com>

> > Cc: Fam Zheng <famz@redhat.com>

> > Cc: Gerd Hoffmann <kraxel@redhat.com>

> > Cc: John Snow <jsnow@redhat.com>

> > Cc: Stefan Hajnoczi <stefanha@gmail.com>

> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>

> > ---

> > 

> > Notes:

> >     I think I managed to incorporate everyone's feedback!

> > 

> >  scripts/git.orderfile | 15 +++++++++++++++

> >  1 file changed, 15 insertions(+)

> >  create mode 100644 scripts/git.orderfile

> > 

> > diff --git a/scripts/git.orderfile b/scripts/git.orderfile

> > new file mode 100644

> > index 000000000000..600a2e4fc540

> > --- /dev/null

> > +++ b/scripts/git.orderfile

> > @@ -0,0 +1,15 @@

> > +*.txt

> 

> We also have *.md. (OK, OK, we have a single docs/bitmaps.md, but maybe

> it's going to be more in the future. :-))


Maybe just insert 'docs/*' here? I'm not sure if it works for the subdirs,
though.

Fam

> 

> Max

> 

> > +configure

> > +GNUmakefile

> > +makefile

> > +Makefile

> > +*.mak

> > +qapi-schema*.json

> > +qapi/*.json

> > +include/qapi/visitor.h

> > +include/qapi/visitor-impl.h

> > +scripts/qapi.py

> > +scripts/*.py

> > +*.h

> > +qapi/qapi-visit-core.c

> > +*.c

> > 

> 

>
Eric Blake Dec. 2, 2016, 4:39 p.m. UTC | #6 
On 12/02/2016 08:41 AM, Fam Zheng wrote:

>>> +++ b/scripts/git.orderfile

>>> @@ -0,0 +1,15 @@

>>> +*.txt

>>

>> We also have *.md. (OK, OK, we have a single docs/bitmaps.md, but maybe

>> it's going to be more in the future. :-))

> 

> Maybe just insert 'docs/*' here? I'm not sure if it works for the subdirs,

> though.


Yes, you can specify subdirs. I think the way git works is that for each
line in the orderfile, it finds all matches in the set of files that
have a diff, then progresses to the next line on any remaining files.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
diff mbox

Patch

diff --git a/scripts/git.orderfile b/scripts/git.orderfile
new file mode 100644
index 000000000000..600a2e4fc540
--- /dev/null
+++ b/scripts/git.orderfile
@@ -0,0 +1,15 @@ 
+*.txt
+configure
+GNUmakefile
+makefile
+Makefile
+*.mak
+qapi-schema*.json
+qapi/*.json
+include/qapi/visitor.h
+include/qapi/visitor-impl.h
+scripts/qapi.py
+scripts/*.py
+*.h
+qapi/qapi-visit-core.c
+*.c