Re: [lwip-devel] Question regarding commit "Test / RFC: Reformat a few files using clang-format"

[Axel Lin]

2018-07-18 14:29 GMT+08:00 Dirk Ziegelmeier <address@hidden>:
> On Wed, Jul 18, 2018 at 8:01 AM Axel Lin <address@hidden> wrote:
>>
>> Hi list,
>>
>> I'm wondering if it's a good timing to add such reformat after v2.1.0rc1.
>
> I just discussed this, I think I'll revert it because we a not sure whether
> we trust clang-format "well enough" before a release not to introduce any
> strange bugs by reformatting (or bugs in reformatting)....
>
>>
>> A few concerns:
>> 1. I guess many out-of-tree projects using lwIP will have a lot of
>> merge conflict after this commit.
>
> Thats unfortunately general to reformatting, yes :-(
>
>>
>> 2. It's not clear to me about the lwIP's coding style.
>>    When I patch the code, I usually follow the original user's coding
>> style
>>    But I seem don't get the rule of clang-format coding style.
>>    e.g. now I found there are 2 space for comment after "#else"
>>    I'm wondering if any document about the coding style changes.
>>    Is there any way to check if a patch follow the lwIP coding style?
>>    Otherwise, there will be many coding style fixes.
>
> lwIP does not really have a clear, written-down coding style. At our
> company, I am currently discussing with Simon what we want to do with our
> in-house code. What coding style should we use? We came to the decision that
> we don't really care about the actual style, and it is more important to us
> to have a tool that enforces the coding style. While coding, we do not want
> worry about coding style. It's convenient to have a tool that reformats the
> code when saving a file.
>
> I guess this is a pragmatic approach, and I think this would also be best
> for lwIP. Fixing coding-style by hand is too much work, neither Simon nor I
> want to do it. I tried astyle some time ago, but it did not work well.
> clang-format seems to do the job quite well. The drawback is that you do not
> have any freedom in formatting any more except when using /* clang-format
> off */ comments.
> I tried to write a clang-format config that matches the "observed" coding
> style of lwIP "close enough" (but I'm always open to suggestions!). lwIP's
> coding style would then be indirectly defined by the behavior of
> clang-format with the .clang-format config file. This is not optimal, but a
> consequence of the "pragmatic approach".
>
> Please, Axel and others, tell me your opinion concerning the lines above!

Hi Dirk,
Here is my humble opinion:

1. The lwIP coding style is documented in doc/contrib.txt
   2.1 Source code style
   2.2 Source code documentation style

   If the code follow the source code style in 2.1/2.2, it should be fine.
   However, the clang-format commit seems add some more extra rules to
the coding style.
   e.g. the number of characters per line, the while space changes, etc.
   I think if you want to apply more rules, you need to update the 2.1
section first.

2. If lwIP want to enforce the consistent coding style, I think you
need a tool to check
   the patch and source code file.
   I used to work on linux kernel driver, and I found the
scripts/checkpatch.pl tool is
   very uselful. I know the maintainer won't accept patch if
checkpatch.pl complains.
   So even I might just read the coding style document for 1 or 2
times, I can easily
   prepare a patch with good coding style by just checking if
checkpatch.pl complains.
   Maybe such coding style checking tool is required by lwIP.

3. I do care about conflicts, so I would suggest improving the coding
style gradually.
   i.e.
   - Ensure new patches with good coding style.
   - Ensure new files with good coding style.
   - But don't enforce coding style for all source code with big-diff.
     (Also note if there is a mistake in .clang-format, you will have
another big-diff to update the code which is not good).
     One topic per patch. You might find maybe some of the code just
leave it as is if you are half-half about the changes.


Regards,
Axel