You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/03/17 22:15:16 UTC

[GitHub] [incubator-nuttx] adamfeuer opened a new pull request #583: [Do Not Merge] experimental clang-format code-formatting tool

adamfeuer opened a new pull request #583: [Do Not Merge] experimental clang-format code-formatting tool
URL: https://github.com/apache/incubator-nuttx/pull/583
 
 
   ### Summary
   
   * **Do Not Merge**
   * Experimental clang-format code formatting tool configuration
   * Changes to code formatting in `sched/` included for discussion as per @davids5 
   
   ### Impact
   
   * adds `.clang-format` file
   * changes code formatting under `sched/` using `clang-format-9` and the config that is checked in here
   * fixes some code formatting and makes other things worse
   * this PR is only for discussion
   
   ### Limitations / TODO
   
   * Lots of limitations and TODOs. I'll add comments from the mailing list in additional comments below.
   
   ### Detail
   
   * See the `tools/format/README.txt` file for info on installing and running `clang-format-9`
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] adamfeuer commented on issue #583: [Do Not Merge] experimental clang-format code-formatting tool

Posted by GitBox <gi...@apache.org>.

adamfeuer commented on issue #583: [Do Not Merge] experimental clang-format code-formatting tool
URL: https://github.com/apache/incubator-nuttx/pull/583#issuecomment-600361350

@davids5 here are some comments from Greg:

* It is destroying indentation randomly (like line 71 of clock.h).
* It is destroying the careful alignment of prototypes like line 90
and beyond.
* It removes all vertical alignment, some optional and some required.
* This is crazy. It moved a left bracket onto the same line as a
definition (line 54 of clock_dow.c)
* in clock_systimer.c, pre-processor definitions must not be indented
when use in the code section (numerous places).
https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#indentation
* in clock_systimespec.c, line 134. The indentation was correct. The
tool ruined it. Also line 142 it again destroys the proper
alignment of the first line of C comment blocks. Numerous other places.
* in clock_timekeeping.c, line 88 abd line 105 spurious destruction of
vertical alignment.
* And on an on.. There is too much there to review.

<hr>

Some more comments from Greg:

I mentioned in the previous email that the tools was screwing up
vertifcal alignment is places where it is recommend and in places where
is it required. Here is an example of the later:

* sched/irq/irq.h
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-08538ebd3dc53f5f85f131c24c84be9d>,
beginning at line 75 all veritical alignment of right hand comments
lost.

A few more:

* clock_timespec_subtract.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-906c272ac7c0b95506056c0c431ab276>,
insanity at line 69. Similar nonsense at group_setuid.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-f39b80ac9e4440e2d2889ef55425f3ef>
line 76, group_signal.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-750c9acf01eefd4c69e3186ceb9519bd>
, line 90 and other places
* nx_smpstart.c,
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-8195c6cbcb792a1f2e6b337039a1d111>line
120, removes required spacing after semi-colon
* It seems to screw up the indentation after any conditional logic.
Like nx_bringup.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-9aa86408017fde10647798c88d4c62b0>,
line 79-110
* group_malloc.c,
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-6cb24cb106c424372fc4dd44e55b8a07>line
51, screw sup indentation of conditional. also group_zalloc.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-87884d55782f00d26a012863add30407>,
line 47 and other places
* group_setupidlefiles.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-aa3bb2f5a7f4db9b96218e96098ea53c>,
line 118 more destruction of indentation. Also nx_bringup.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-9aa86408017fde10647798c88d4c62b0>,
line 74 and other places
* nx_start.c
<https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-e36621b38d26a5eeb9f0c9c779931e7b>
, all braces trashed beginning at line 253. Right alignment of all
comments lost.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [incubator-nuttx] PetervdPerk-NXP commented on issue #583: [Do Not Merge] experimental clang-format code-formatting tool

Posted by GitBox <gi...@apache.org>.

PetervdPerk-NXP commented on issue #583: [Do Not Merge] experimental clang-format code-formatting tool
URL: https://github.com/apache/incubator-nuttx/pull/583#issuecomment-605943351
 
 
   > This is crazy. It moved a left bracket onto the same line as a
   definition (line 54 of clock_dow.c)
   
   This a weird clang-format thing but if you add a trailing comma in the initializer list, clang-format will format it correctly.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] adamfeuer commented on pull request #583: [Do Not Merge] experimental clang-format code-formatting tool

Posted by GitBox <gi...@apache.org>.

adamfeuer commented on pull request #583:
URL: https://github.com/apache/incubator-nuttx/pull/583#issuecomment-635647112


   @patacongo Thanks– I will take it up in the future again, right now I'm working on the SAMA5D27 SDMMC driver and that's taking all my NuttX time at the moment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx] patacongo closed pull request #583: [Do Not Merge] experimental clang-format code-formatting tool

Posted by GitBox <gi...@apache.org>.

patacongo closed pull request #583:
URL: https://github.com/apache/incubator-nuttx/pull/583


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #583: [Do Not Merge] experimental clang-format code-formatting tool

Posted by GitBox <gi...@apache.org>.

patacongo edited a comment on pull request #583:
URL: https://github.com/apache/incubator-nuttx/pull/583#issuecomment-635644673


   @adamfeuer I am closing this for the time being.  If you need to work on it further in the future, please feel free to re-open it.  I would just like to get it out of the way to make managing the PRs easier.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx] patacongo commented on pull request #583: [Do Not Merge] experimental clang-format code-formatting tool

Posted by GitBox <gi...@apache.org>.

patacongo commented on pull request #583:
URL: https://github.com/apache/incubator-nuttx/pull/583#issuecomment-635644673


   @adamfeuer I am closing this for the time being.  If you need to work on it further in the future, please feel free to re-open it.  I would just like to get it out of the way to make managin the PRs easier.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org