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

7 messages, 5 authors, 2017-01-20 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2017-01-16 00:35:28

Wolfram Sang [off-list ref] 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.

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

From: Jacob Keller <hidden>
Date: 2017-01-16 04:05:50

On Sun, Jan 15, 2017 at 4:35 PM, Junio C Hamano [off-list ref] 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

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

From: Wolfram Sang <hidden>
Date: 2017-01-19 20:44:34

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.
Sadly, no further responses so far. Let's see if they will come if I
keep posting my pull requests with that information attached.
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 didn't know about trailers before. As I undestand it, I could use
"Tested-by" as the key, and the commit subject as the value. This list
then could be parsed and brought into proper output shape. It would
simplify the subject parsing, but most things my AWK script currently
does would still need to stay or to be reimplemented (extracting names
from tags, creating arrays of tags given by $name). Am I correct?

All under the assumption that trailers work on a range of commits. I
have to admit that adding this to git is beyond my scope.

Thanks for looking into it,

   Wolfram

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

From: Jeff King <hidden>
Date: 2017-01-19 21:47:25

On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang wrote:
quoted
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 didn't know about trailers before. As I undestand it, I could use
"Tested-by" as the key, and the commit subject as the value. This list
then could be parsed and brought into proper output shape. It would
simplify the subject parsing, but most things my AWK script currently
does would still need to stay or to be reimplemented (extracting names
from tags, creating arrays of tags given by $name). Am I correct?

All under the assumption that trailers work on a range of commits. I
have to admit that adding this to git is beyond my scope.
This sounds a lot like the shortlog-trailers work I did about a year
ago:

  http://public-inbox.org/git/20151229073832.GN8842@sigill.intra.peff.net/

  http://public-inbox.org/git/20151229075013.GA9191@sigill.intra.peff.net/

Nobody seemed to really find it useful, so I didn't pursue it.

Some of the preparatory patches in that series bit-rotted in the
meantime, but you can play with a version based on v2.7.0 by fetching
the "shortlog-trailers-historical" branch from
https://github.com/peff/git.git.

And then things like:

  git shortlog --ident=tested-by --format='...tested a patch by %an'

work (and you can put whatever commit items you want into the --format,
including just dumping the hash if you want to do more analysis).

-Peff

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

From: Jacob Keller <hidden>
Date: 2017-01-19 23:43:03

On Thu, Jan 19, 2017 at 1:20 PM, Jeff King [off-list ref] wrote:
On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang wrote:
quoted
quoted
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 didn't know about trailers before. As I undestand it, I could use
"Tested-by" as the key, and the commit subject as the value. This list
then could be parsed and brought into proper output shape. It would
simplify the subject parsing, but most things my AWK script currently
does would still need to stay or to be reimplemented (extracting names
from tags, creating arrays of tags given by $name). Am I correct?

All under the assumption that trailers work on a range of commits. I
have to admit that adding this to git is beyond my scope.
This sounds a lot like the shortlog-trailers work I did about a year
ago:

  http://public-inbox.org/git/20151229073832.GN8842@sigill.intra.peff.net/

  http://public-inbox.org/git/20151229075013.GA9191@sigill.intra.peff.net/

Nobody seemed to really find it useful, so I didn't pursue it.

Some of the preparatory patches in that series bit-rotted in the
meantime, but you can play with a version based on v2.7.0 by fetching
the "shortlog-trailers-historical" branch from
https://github.com/peff/git.git.

And then things like:

  git shortlog --ident=tested-by --format='...tested a patch by %an'

work (and you can put whatever commit items you want into the --format,
including just dumping the hash if you want to do more analysis).

-Peff
This sounds interesting to me! When I have some more time to take a
look at this i might see if I can revive it.

Thanks,
Jake

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

From: Joe Perches <joe@perches.com>
Date: 2017-01-20 00:07:29

On Thu, 2017-01-19 at 15:42 -0800, Jacob Keller wrote:
On Thu, Jan 19, 2017 at 1:20 PM, Jeff King [off-list ref] wrote:
quoted
On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang wrote:
quoted
quoted
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 didn't know about trailers before. As I undestand it, I could use
"Tested-by" as the key, and the commit subject as the value. This list
then could be parsed and brought into proper output shape. It would
simplify the subject parsing, but most things my AWK script currently
does would still need to stay or to be reimplemented (extracting names
from tags, creating arrays of tags given by $name). Am I correct?

All under the assumption that trailers work on a range of commits. I
have to admit that adding this to git is beyond my scope.
This sounds a lot like the shortlog-trailers work I did about a year
ago:

  http://public-inbox.org/git/20151229073832.GN8842@sigill.intra.peff.net/

  http://public-inbox.org/git/20151229075013.GA9191@sigill.intra.peff.net/

Nobody seemed to really find it useful, so I didn't pursue it.

Some of the preparatory patches in that series bit-rotted in the
meantime, but you can play with a version based on v2.7.0 by fetching
the "shortlog-trailers-historical" branch from
https://github.com/peff/git.git.

And then things like:

  git shortlog --ident=tested-by --format='...tested a patch by %an'

work (and you can put whatever commit items you want into the --format,
including just dumping the hash if you want to do more analysis).

-Peff
This sounds interesting to me! When I have some more time to take a
look at this i might see if I can revive it.
Can the terminology please be standardized to what
was once called bylines?

https://patchwork.kernel.org/patch/9307703/

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

From: Jacob Keller <hidden>
Date: 2017-01-20 00:14:22

On Thu, Jan 19, 2017 at 4:06 PM, Joe Perches [off-list ref]
wrote:>> This sounds interesting to me! When I have some more time to
take a
quoted
look at this i might see if I can revive it.
Can the terminology please be standardized to what
was once called bylines?

https://patchwork.kernel.org/patch/9307703/
I am fairly certain we've settled on "trailers" at this point. I don't
have an objection to either name, but most of the code today uses
trailers.

Thanks,
Jake
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help