<div dir="ltr">Hi Paul,<div>  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.  </div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.<br></div><div><br></div><div>Martin</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 23, 2017 at 2:16 PM, Paul Menzel via coreboot <span dir="ltr"><<a href="mailto:coreboot@coreboot.org" target="_blank">coreboot@coreboot.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Dear coreboot folks,<br>
<br>
<br>
Each commit pushed to Gerrit is automatically tested for “formal”<br>
issues by using `<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a>`. See for example [1].<br>
<br>
Though despite missing a space violating our coding style, which is<br>
also found by `<a href="http://checkpatch.pl" rel="noreferrer" target="_blank">checkpatch.pl</a>` [2], the comment contains, that no errors<br>
is found.<br>
<br>
> <a href="https://qa.coreboot.org/job/coreboot-checkpatch/5142/" rel="noreferrer" target="_blank">https://qa.coreboot.org/job/<wbr>coreboot-checkpatch/5142/</a> : SUCCESS<br>
<br>
```<br>
ERROR: space required before the open brace '{'<br>
#49: FILE: src/mainboard/google/gru/pwm_<wbr>regulator.c:61:<br>
+       } else if (IS_ENABLED(CONFIG_BOARD_<wbr>GOOGLE_KEVIN) && board_id() >= 6){<br>
<br>
total: 1 errors, 0 warnings, 49 lines checked<br>
```<br>
<br>
Is there a reason for not relaying these errors?<br>
<br>
If not, it’d be great to do so (and for the Chromium and Intel folks to<br>
also do that in their repository).<br>
<br>
<br>
Thanks,<br>
<br>
Paul<br>
<br>
<br>
[1] <a href="https://review.coreboot.org/18460" rel="noreferrer" target="_blank">https://review.coreboot.org/<wbr>18460</a><br>
[2] <a href="https://qa.coreboot.org/job/coreboot-checkpatch/5142/console" rel="noreferrer" target="_blank">https://qa.coreboot.org/job/<wbr>coreboot-checkpatch/5142/<wbr>console</a><br>--<br>
coreboot mailing list: <a href="mailto:coreboot@coreboot.org">coreboot@coreboot.org</a><br>
<a href="https://www.coreboot.org/mailman/listinfo/coreboot" rel="noreferrer" target="_blank">https://www.coreboot.org/<wbr>mailman/listinfo/coreboot</a><br></blockquote></div><br></div>