[U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch

[Albert ARIBAUD]

Le 16/04/2011 08:18, Macpaul Lin a écrit :
> HI all,
>
> 2011/4/15 Mike Frysinger<vapier at gentoo.org>:
>>>> up to Wolfgang how he feels about ifdef indentation
>>>
>>> In this specific case of #ifdef indentation I feel that the original
>>> form (which causes checkpatch warnings) is actually easier to read, so
>>> I tend to keep it.  But I am aware that this is inconsequent as we ask
>>> for "indentation by TAB only" everywhere else.
>
> For this specific #ifdef case, I will agree to keep the origin format,
> it's exactly easier to read.
> Because I'm not in the office, I can only send the fixed patch v2 until 4/18.
>
>> well, this is a case where i would say a soft rule is OK -- i.e. allow both
>> options and let the active maintainer of the code in question decide.  i too
>> prefer the current style as i find it easier to read, but if someone
>> maintaining code i never work on wants to be strict about tabs, then it's no
>> sweat off my back.
>>
>> so, not to dupe another thread, but i'd say "use common sense".  but that
>> probably too isn't consistent/clear enough for many people.
>> -mike
>
> Yes, use common sense is really too lose for many people.
>
> Checkpatch is really help for people to maintain the consistence for
> coding style and also avoid the coding method which might lead logic
> failure.
> However, the drawback is that we still have effort to think about if
> the warning should be correct up according to every case that has been
>   reported by checkpatch.
>
> For example, 80 charecters is the most often case. Sometimes the code
> is really easier to be read in the single one line. Sometimes the
> complicated code that should be execute in one line is really hard to
> be modified into short format to avoid exceeding the 80 charecters
> lenght rule. Although most of time people can decide which format is
> better for human reading than checkpatch's parsing result.
> We still have effort to inform the patch commiters to modified the
> code and discuss with it.
>
> Also, the coding style clean up for the old code already exist in git
> repository will help the new contributers and reduce the disscusion
> effort passively.
> That's why I think we can do clean up for old code slowly when someone
> have time.
>
> Maybe list a exception form and some soft rules on the web page is
> sometimes help to the contributers just new to the u-boot project.

Ack to the whole of this. Until there is such a list, the casual patch 
committers will either not apply checkpatch, or apply a zero-warning 
strategy.

> I have an idea to recruit some colledgue students locally as
> volunteers to clean up the old code according to "checkpatch". They
> just do code clean up. By doing this activity will also make them be
> familiar with the open source projects. At the same time they can also
> be familiar with git CVS. Of course a guideline for such code clean up
> activity must be work out first.
>
> Don't know what do you think of it?

Maybe your students could successfully get some U-boot-friendly patches 
applied to checkpatch? I tried some time ago for an option to control 
the line length, but gave up because there seemed to be no reaction -- 
see <https://patchwork.kernel.org/patch/149941/>. Having someone who's 
job description actually includes "nagging the checkpatch maintainer 
until the frigging patches are accepted" could help.

Maybe a better option than adding ad hoc options would be a single patch 
to make checkpatch.pl load a custom set of checking rules that could 
supersede the original ones. The set could be contained in the current 
directory as ./.checkpatch.rules (and maybe also look in $(HOME) if not 
found in $(PWD). Not too sure this is technically feasible, though.

> Thanks.

Amicalement,
-- 
Albert.