[coreboot] QA: Properly relay results of `checkpatch.pl` check

Paul Menzel paulepanter at users.sourceforge.net
Fri Feb 24 09:13:47 CET 2017


Dear Martin,


Am Donnerstag, den 23.02.2017, 16:47 -0700 schrieb Martin Roth:

> checkpatch is currently not a gating item in jenkins and should always
> pass right now.  The checkpatch build was added to jenkins to allow people
> to see at the results of the console output for the patch without having to
> download and run checkpatch themselves.
> 
> Unfortunately, checkpatch is a lot stricter than we want to be.  It would
> require someone who fixes a misspelling on a line to also fix any other
> mistakes on that line.  Because of this, we don't want to fail the patch
> based on checkpatch errors styleguide issues.  We had discussed adding a
> non-failing 'lint' flag to gerrit to notify people that checkpatch was
> failing so that they could go look at it, but due to people's workloads,
> this hasn't happened yet.  Honestly, it's probably fallen off the radar.

The non-failing option, would indeed be a good compromise. I believe,
that right now the *SUCCESS* message is confusing, letting most people
think, that there are *no* problems. Could the message be adapted, or
is that a Gerrit/Jenkins limitation?

> Personally, I still think it's a bad idea to refuse patches that don't pass
> checkpatch, but I'd be glad to discuss it.

The other option would be, to make it a “gating item”, and see how many
of the unwanted situations occur. If it’s only a few, then the change-
set owner, could just add a comment, that it’s a such a situation
(“false positive”), and the Gerrit administrators could override the
result.

It’d be interesting, if that will be less work in the end, as less
reviewer time is spent on commenting the formal issues.

> Also, the error you mentioned WAS noticed, and fixed in a follow-on patch
> so that it could be pulled back into the chromium tree.

Yes, I know. I am sorry for not making that more clear in my message.


Thanks,

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: This is a digitally signed message part
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20170224/eedf0c4c/attachment.asc>


More information about the coreboot mailing list