
Dear "Moffett, Kyle D",
In message AC0C0781-9E5F-42F7-9DB6-EECF6A5BE840@boeing.com you wrote:
Just looking at the last ~200 commits (actually 187, because it ignores mer= ges):
$ git format-patch -o recent-patches -200 origin/master $ ./checkpatch.pl --no-tree --strict recent-patches/* >checkpatch.log 2>&1 $ grep 'over 80 char' checkpatch.log | wc -l 130
That's 130 lines in the last 200 patches which are over 80 characters?!?! How are those patches any different from mine?
The difference is: They were not detected.
Patches welcome.
Look, I'm really trying to comply with U-Boot coding standards, but I'm rea= lly of pissed off about the inconsistent requirements you are applying to my patches versus a lot of other things that YOU ARE MERGING on a regular basi= s.
The requirements are NOT inconsistent. It's just that nobody is perfect, and nobody ever claimed that we manage to get 100% of review coverage.
So why are you picking on my board-specific code so hard here? This is
It's just that the problems got noticed.
"checkpatch.pl --no-tree"
What version of checkpatch are you running? I copied version 0.31 out of my latest Linux kernel tree, which identical to the latest version from here:
-> checkpatch.pl --version Usage: checkpatch.pl [OPTION]... [FILE]... Version: 0.31 ...
If U-Boot policy is to run checkpatch then you'd better either specify a particular version and command-line options or be willing to accept the default output of the latest version.
I don't see any version issue here, nor do I use any non-standard options.
+U_BOOT_CMD(
- hww1u1a_test_cpu_a, 1, 0, do_hww1u1a_test_cpu_a,
- "Test if this is CPU A (versus B) on the eXMeritus HWW-1U-1A board",
- /* */" && <command-if-true>\n"
- "hww1u1a_test_cpu_a || <command-if-false>\n"
=20 What is this empty comment needed for?
Just a mental placeholder for the fact that the U_BOOT_CMD macro inserts the name of the command in that spot. Will remove.
We don't provide usage examples in the help text. This should be fixed in the first place.
This *IS* the help text, and not a sample usage. This is visually and effectively no different from this text in common/cmd_mp.c:
The synopsis of a command gives the command name and possible options, and explains what the command does and what the options do.
"name && <command-if-true>" does NOT fall into that pattern.
Look, alternatively I can claim your help message is highly incomplete as it fails to cover use cases like
name || <command-if-false>
etc.
Not to mention that the usage message is plain wrong for all boards that don't have the hush shell enabled.
Best regards,
Wolfgang Denk