Skip to contentSkip to conversation

checkpatch.pl checking

U-Boot Tom Rini 2 participants 2 messages Jan 15, 2018 – Jan 16, 2018

lore
Share & tools
lore

Tom Rini

author
#0 lore raw

Hey all,

As came up in another thread, yes, we do care about what checkpatch.pl says. I've been lax about it as I'd not seen a good way ot integrate it when I looked last. Turns out it's easier to integrate now with the --git arg, so I've got that in my scripts now. Since it would be unfair to reject PRs due to checkpatch.pl issues today, I'm not rejecting, I'm just pointing out the problems it sees. And there will be false positives as sometimes it's fine to go more than 80 chars wide for example (dts files, pinmux stuff). But in general, it shouldn't be complaining, so please take a look. Thanks all!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180115/4963a65d/attachment.sig>
#1 lore raw

Hello Tom,

Am 15.01.2018 um 22:50 schrieb Tom Rini:

Quoted 11 lines

Hey all,

As came up in another thread, yes, we do care about what checkpatch.pl says. I've been lax about it as I'd not seen a good way ot integrate it when I looked last. Turns out it's easier to integrate now with the --git arg, so I've got that in my scripts now. Since it would be unfair to reject PRs due to checkpatch.pl issues today, I'm not rejecting, I'm just pointing out the problems it sees. And there will be false positives as sometimes it's fine to go more than 80 chars wide for example (dts files, pinmux stuff). But in general, it shouldn't be complaining, so please take a look. Thanks all!

I check my patches in my patchwork todo list automatically with tbot in my weekly automated U-Boot mainline tests. For each patch checkpatch.pl script is called (but tbot does not fail, if errors found, but you can configure tbot to stop on checkpatch failures ...)

Last test from this night (for example for the corvus board):

http://xeidos.ddns.net/tests/test_db_auslesen.php

And as a good example, there are 2 checkpatch failures for patches on my ToDo list!

http://xeidos.ddns.net/tbot/id_590/tbot.txt

search for "wget http://patchwork" and you can see the wget command for each patch in my patchworktodo list, and the following checkpatch call (and the following git am ...)

The test fails, as the last patch does not apply to u-boot mainline, and this is an error which tbot stops ... just restarted this test after I reviewed the 2 patches, set them to patchwork state "changes requested", so they are not visible in the next test.

I only have to look on tuesday morning, if my results webpage has only new green lines ... if not I have to look deeper.

If all is green, I scan for checkpatch errors in the log and decide if I want to accept this errors/warnings (or not), than I review the patch ...

Also, I can be sure, U-Boot code compiles clean with this patches, and do not break my boards (do some cmdline tests, also call u-boots test/py framework) ... so I can concentrate on review tasks ...

After all is fine, I send the tree to my github account, and trigger with this a travis build ... and if there all is clean, you get a pull request ... the boring tasks are automated, only reviewing is human work :-D

bye,
Heiko

Signature · 3 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de