[U-Boot] [PATCH v6 4/4] mpc85xx: Add board support for the eXMeritus HWW-1U-1A devices

[Moffett, Kyle D]

On Mar 21, 2011, at 18:24, Wolfgang Denk wrote:
> In message <DDD31591-33CD-49E4-B303-3477E00933BF at boeing.com> you wrote:
>> 
>> I apparently cannot rely on the U-Boot *CODE* to understand what the
>> U-Boot *CODING* style is.
> 
> You don't have to rely on the code.  It's clearly documented.
> 
> The README says:
> 
> 	Coding Standards:
> 	-----------------
> 
> 	All contributions to U-Boot should conform to the Linux kernel
> 	coding style; see the file "Documentation/CodingStyle" and the script
> 	"scripts/Lindent" in your Linux kernel source directory...
> 
> http://www.denx.de/wiki/U-Boot/CodingStyle  says the same.

So here it is claimed that the U-Boot coding style is the same as the Linux
Kernel coding style.

But previously you said:
>>>> WARNING: line over 80 characters
>>>> #463: FILE: board/exmeritus/hww1u1a/hww1u1a.c:136:
>>>> +int do_hww1u1a_test_cpu_a(cmd_tbl_t *cmdtp, int flag, int argc, char *
>>>> const argv[])
>>> 
>>> This one is only a warning, and it's much more readable to have 1
>>> 84-character line than split it across 2 different lines.  Even still,
>>> this warning is only issued if you pass "--subjective" to checkpatch, which
>>> is documented to "enable more subjective tests".
> 
> In U-Boot, it is considered to be an ERROR.


So which is it?

If U-Boot uses the official Linux Kernel CodingStyle, then a few >80-char
lines are OK if it increases readability, for example by not having to wrap
a simple U_BOOT_CMD function declaration that goes just a whole 4 characters
over the limit.

If U-Boot does NOT use the Linux Kernel CodingStyle and wants to refuse all
patches with over 80 characters then you should copy checkpatch.pl and the
CodingStyle document and change that wording from "strongly preferred" to
"hard requirement", and change the "WARNING" to "ERROR".


> And the referred document says:
> 			Chapter 2: Breaking long lines and strings
> 	 
> 	Coding style is all about readability and maintainability using
> 	commonly
> 	available tools.
> 	 
> 	The limit on the length of lines is 80 columns and this is a strongly
> 	preferred limit.
> 	 
> 	Statements longer than 80 columns will be broken into sensible chunks.
> 
> Now what exactly is unclear here?

You removed the very next paragraph in the Linux CodingStyle file,
which contains:

> The only exception to this is where exceeding 80 columns significantly increases

> readability and does not hide information.


Furthermore, Linus Torvalds himself said in an email:
>> We fixed that to allow checkpatch to skip those warnings, but the fact is, 
>> the fundamnetal problem has always been the "80 character" part.

>> 
>> I don't think any kernel developers use a vt100 any more. And even if they 
>> do, I bet they curse the "24 lines" more than they curse the occasional 
>> 80+ character lines.
>> 
>> I'd be ok with changing the warning to 132 characters, which is another 
>> perfectly fine historical limit. Or we can split the difference, and say 
>> "ok, 106 characters is too much". I don't care. But 80 characters is 
>> causing too many idiotic changes.
>> 
>> There are way worse problems in many patches than long lines. Too complex 
>> expressions. Too deep indentation. Pure crap code. People seem to get way 
>> too hung up on ".. but at least it passes checkpatch". 

The line you were complaining about is a static U_BOOT_CMD function declaration
for goodness sakes!  It's about as common as dirt in the U-Boot code and the
only reason it doesn't fit on one line anymore is because I made the name of the
function *NAME* about 8 characters longer in this version of the patch than it
was in a previous patch.

It's not any more complex than it was before, nor is it any harder to read.

I'm tired and fed up with this whole mess.  If you think it's likely to be
accepted I'll go ahead and submit one more final respin tomorrow, assuming
I feel up to it.  Otherwise, I wish you the best of luck with U-Boot.

Good night.

Cheers,
Kyle Moffett