Re: [PATCH 0/25] detecting &&-chain breakage

From: Jeff King <peff@peff.net>
Date: Mar 21, 2015, 22:23 UTC
Message-ID: <20150321222304.GA27850@peff.net>
In-Reply-To: <xmqq384ydybc.fsf@gitster.dls.corp.google.com>

On Sat, Mar 21, 2015 at 11:01:43AM -0700, Junio C Hamano wrote:

Show 11 quoted lines

> > Running:
> >
> >   git diff origin origin/jk/test-chain-lint |
> >   perl diff-blame.pl jk/test-chain-lint |
> >   grep EOF
> >
> > was fun. At least I am not the only one. :)
> 
> The parameter to diff-blame.pl should be origin, instead of
> jk/test-chain-lint, I presume?  You are grabbing the preimage line
> numbers and asking blame to find out who wrote them.

Yes, sorry, that was an error translating from what I actually ran in the shell into the email. It should be "origin". And if the script really wanted to be user-friendly, it should probably take two endpoints and just run the diff itself (when I started it, I assume that you could process any diff, but of course you must know the start point to get a reasonable blame).

Show 6 quoted lines

> > Nor the worst in the "severe" category.
> 
> I do not quite get what this means---the script does not seem to
> judge what is severe and what is not, so I presume that this is to
> be judged by whoever is reading the output from the above pipeline
> after replacing "grep EOF" with "less" or something?

That was the exercise I left to the reader. :) In this case, it is possible because I have already split the patches into "severe", "moderate", and "trivial" cases, so you can blame only the severe patch (using its parent as the start-point).

> > while (<STDIN>) {
> >   if (m{^--- .*?/(.*)}) {
> 
> This may match a removal of a line that begins with "^-- something/" ;-)

True. I was trying to avoid being stateful in my diff parsing. I guess it would be enough to parse the hunk headers to know how many lines are in the hunk. Not worth it for this one-off, but a good thing if somebody wanted to pick this idea up for a "real" tool.

> >   # XXX coalesce blocks of adjacent lines into ranges?
> >   system(qw(git --no-pager blame), @ARGV,
> 
> You may want to pass an option to always show the filename here.

I left that to the user. I actually found "--line-porcelain" useful for gathering statistics (e.g., piped to "grep '^author ' | sort | uniq -c").

-Peff