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

Groups > linux.kernel > #1563071 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC for GIT] pull-request: add praise to people doing QA Wolfram Sang <wsa@the-dreams.de> - 2017-01-19 21:50 +0100
    Re: [RFC for GIT] pull-request: add praise to people doing QA Junio C Hamano <gitster@pobox.com> - 2017-01-19 22:30 +0100
      Re: [RFC for GIT] pull-request: add praise to people doing QA Wolfram Sang <wsa@the-dreams.de> - 2017-01-19 23:10 +0100
    Re: [RFC for GIT] pull-request: add praise to people doing QA Jeff King <peff@peff.net> - 2017-01-19 22:50 +0100
      Re: [RFC for GIT] pull-request: add praise to people doing QA Jacob Keller <jacob.keller@gmail.com> - 2017-01-20 00:50 +0100
        Re: [RFC for GIT] pull-request: add praise to people doing QA Joe Perches <joe@perches.com> - 2017-01-20 01:10 +0100
          Re: [RFC for GIT] pull-request: add praise to people doing QA Jacob Keller <jacob.keller@gmail.com> - 2017-01-20 01:20 +0100

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

> 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

[toc] | [next] | [standalone]

#1563106

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

> 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?

That is not exactly what I had in mind.  I was wondering if we can
do without any external script, implementing the logic you added
inside shortlog with an extra option that triggers the whole thing,
which may call into the same trailers API as used by the
interpret-trailers command to do the parsing and picking out parts.

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

#1563126

[Multipart message — attachments visible in raw view] — view raw

> > 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?
> 
> That is not exactly what I had in mind.  I was wondering if we can
> do without any external script, implementing the logic you added
> inside shortlog with an extra option that triggers the whole thing,
> which may call into the same trailers API as used by the
> interpret-trailers command to do the parsing and picking out parts.

Sorry for being unclear. That's what I meant with "or to be
reimplemented". I should have added "in C".

I am afraid this also requires more time than I am willing to
spend on this issue. Seems my hack is going to stay for a while here.

However, thank you for your time and assisting me with pointers!

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

#1563121

On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang 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 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

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

#1563207

On Thu, Jan 19, 2017 at 1:20 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang 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 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

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

#1563210

On Thu, 2017-01-19 at 15:42 -0800, Jacob Keller wrote:
> On Thu, Jan 19, 2017 at 1:20 PM, Jeff King <peff@peff.net> wrote:
> > On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang 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 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/

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

#1563214

On Thu, Jan 19, 2017 at 4:06 PM, Joe Perches <joe@perches.com>
wrote:>> 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/
>

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web