lwip-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lwip-devel] Question regarding commit "Test / RFC: Reformat a few f


From: Dirk Ziegelmeier
Subject: Re: [lwip-devel] Question regarding commit "Test / RFC: Reformat a few files using clang-format"
Date: Wed, 18 Jul 2018 08:29:28 +0200

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!
 
3. The patch is so big (-2681/+2981), so I'm wondering if anyone can
review the change.
   (Just think about would you apply the patch if someone send you a
patch like such big-diff changes.)
    And since it is already committed, it's not just a "Test/RFC".
​That's right, but a commit can be reverted when it is not acceptable. But I just noticed that this still screws up git blame, which I didn't think about when committing! Sorry for that, it was not my intention!!!

​Regards
Dirk


reply via email to

[Prev in Thread] Current Thread [Next in Thread]