Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1559320 > unrolled thread

[RFC for GIT] pull-request: add praise to people doing QA

Back to article view | Back to linux.kernel

Contents

  [RFC for GIT] pull-request: add praise to people doing QA Wolfram Sang <wsa@the-dreams.de> - 2017-01-15 19:40 +0100
    Re: [RFC for GIT] pull-request: add praise to people doing QA Junio C Hamano <gitster@pobox.com> - 2017-01-16 01:40 +0100
      Re: [RFC for GIT] pull-request: add praise to people doing QA Jacob Keller <jacob.keller@gmail.com> - 2017-01-16 05:10 +0100

#1559320 — [RFC for GIT] pull-request: add praise to people doing QA

Asking for opinions on lkml and git...

Getting enough quality assurance is likely one of the bigger upcoming tasks in
the near future. To improve the situation, praise the people already doing that
by adding their names to pull requests in the same manner that patch authors
are credited. Here is an example, I sent out today [1]:

=== old stuff

The following changes since commit a121103c922847ba5010819a3f250f1f7fc84ab8:

...

Vlad Tsyrklevich (1):
      i2c: fix kernel memory disclosure in dev interface

=== new stuff starts here

with much appreciated quality assurance from
----------------------------------------------------------------
Andy Shevchenko (1):
      (Rev.) i2c: piix4: Avoid race conditions with IMC

Benjamin Tissoires (1):
      (Test) i2c: do not enable fall back to Host Notify by default

Vladimir Zapolskiy (1):
      (Rev.) i2c: print correct device invalid address

=== diffstat, ...

This patch is a very early RFC to collect opinions. I am not very familiar with
the git codebase, but I guess using a filter needs to be reworked, the
dependency on GNU awk may be frowned upon (though 'asorti' is really useful
here), the reg-ex are not super-solid, and it should be a command-line option,
of course. That all being said, it was a fast way to produce what I would like
to add to my pull requests for the i2c subsystem and to see if other kernel/git
maintainers are interested in something like this.

Disclaimer: while this patch applies to the git codebase, I have to admit that
I simply patched around in /usr/lib/git-core of my Debian machine :)

So much for now, let me know what you think,

   Wolfram

[1] http://lkml.org/lkml/2017/1/15/55

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 git-praise-qa.awk   |   33 +++++++++++++++++++++++++++++++++
 git-request-pull.sh |    1 +
 2 files changed, 34 insertions(+)

Index: git-2.11.0/git-request-pull.sh
===================================================================
--- git-2.11.0.orig/git-request-pull.sh
+++ git-2.11.0/git-request-pull.sh
@@ -155,6 +155,7 @@ then
 fi &&
 
 git shortlog ^$baserev $headrev &&
+git log --no-merges ^$baserev $headrev | git-praise-qa.awk &&
 git diff -M --stat --summary $patch $merge_base..$headrev || status=1
 
 exit $status
Index: git-2.11.0/git-praise-qa.awk
===================================================================
--- /dev/null
+++ git-2.11.0/git-praise-qa.awk
@@ -0,0 +1,33 @@
+#! /usr/bin/gawk -f
+
+# New commit found, empty subject variable
+/^commit / { subject = "" }
+
+# Grab the subject line
+!subject && /^    / { subject = substr($0, 5); }
+
+# Scan for tags and get the type
+/^    Reviewed-by:/ { type = "Rev." }
+/^    Tested-by:/ { type = "Test" }
+
+type && subject {
+	# Extract the name
+	sub(/^.*: /, ""); sub(/ <.*/, ""); name = $0;
+	# Collect tags given by 'name'
+	tags[name] = tags[name] "      (" type ") " subject "\n";
+	count[name]++;
+	# Done, clear flag
+	type = "";
+}
+
+END {
+	print "\nwith much appreciated quality assurance from"
+	print "----------------------------------------------------------------"
+	# Sort by names
+	asorti(tags, sorted_names);
+	# printout in git style
+	for (i in sorted_names) {
+		name = sorted_names[i];
+		print name " (" count[name] "):" "\n" tags[name];
+	}
+}

[toc] | [next] | [standalone]

#1559379

Wolfram Sang <wsa@the-dreams.de> writes:

> === new stuff starts here
>
> with much appreciated quality assurance from
> ----------------------------------------------------------------
> Andy Shevchenko (1):
>       (Rev.) i2c: piix4: Avoid race conditions with IMC
>
> Benjamin Tissoires (1):
>       (Test) i2c: do not enable fall back to Host Notify by default
>
> Vladimir Zapolskiy (1):
>       (Rev.) i2c: print correct device invalid address
>
> === diffstat, ...
>
> This patch is a very early RFC to collect opinions. I am not very familiar with
> the git codebase, but I guess using a filter needs to be reworked, the
> dependency on GNU awk may be frowned upon (though 'asorti' is really useful
> here), the reg-ex are not super-solid, and it should be a command-line option,
> of course. That all being said, it was a fast way to produce what I would like
> to add to my pull requests for the i2c subsystem and to see if other kernel/git
> maintainers are interested in something like this.
>
> Disclaimer: while this patch applies to the git codebase, I have to admit that
> I simply patched around in /usr/lib/git-core of my Debian machine :)
>
> So much for now, let me know what you think,

So the idea is to have list of those whose names appear on
Reviewed-by: and Tested-by: collected and listed after the list of
commit titles and author names.  I personally do not see much
downsides in doing so, but I do not consume that many PRs myself, so
let's hear from those who actually do process many of them.

As to the implementation, I am wondering if we can make this somehow
work well with the "trailers" code we already have, instead of
inventing yet another parser of trailers.  

In its current shape, "interpret-trailers" focuses on "editing" an
existing commit log message to tweak the trailer lines.  That mode
of operation would help amending and rebasing, and to do that it
needs to parse the commit log message, identify trailer blocks,
parse out each trailer lines, etc.  

There is no fundamental reason why its output must be an edited
original commit log message---it should be usable as a filter that
picks trailer lines of the selected trailer type, like "Tested-By",
etc.

[toc] | [prev] | [next] | [standalone]

#1559433

On Sun, Jan 15, 2017 at 4:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> As to the implementation, I am wondering if we can make this somehow
> work well with the "trailers" code we already have, instead of
> inventing yet another parser of trailers.
>
> In its current shape, "interpret-trailers" focuses on "editing" an
> existing commit log message to tweak the trailer lines.  That mode
> of operation would help amending and rebasing, and to do that it
> needs to parse the commit log message, identify trailer blocks,
> parse out each trailer lines, etc.
>
> There is no fundamental reason why its output must be an edited
> original commit log message---it should be usable as a filter that
> picks trailer lines of the selected trailer type, like "Tested-By",
> etc.

I have been looking at ways to use the interpret-trailers as a way to
filter commits and print out trailers, and this sort of feature would
be useful to me if it were generic. (and then pull-request could use
the generic interface to grab the data and then parse it into a praise
format)

Thanks,
Jake

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web