- 
                Notifications
    
You must be signed in to change notification settings  - Fork 447
 
iox-#1368 Add cmake linter and formatter scripts #1885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
iox-#1368 Add cmake linter and formatter scripts #1885
Conversation
        
          
                iceoryx_hoofs/CMakeLists.txt
              
                Outdated
          
        
      | @@ -1,17 +1,17 @@ | |||
| # Copyright (c) 2019 - 2021 by Robert Bosch GmbH. All rights reserved. | |||
| # Copyright (c) 2020 - 2022 by Apex.AI Inc. All rights reserved. | |||
| # Copyright (c) 2019 - 2021 by Robert Bosch GmbH. All rights reserved. Copyright | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The copyright header should not be reformatted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will need to fence this block from formatting, so disabling the formatter before it and then enable it after
        
          
                iceoryx_hoofs/CMakeLists.txt
              
                Outdated
          
        
      | BUILD_INTERFACE | ||
| ${PROJECT_SOURCE_DIR}/include | ||
| ${PROJECT_SOURCE_DIR}/legacy/include | ||
| ${PROJECT_SOURCE_DIR}/memory/include | ||
| ${PROJECT_SOURCE_DIR}/container/include | ||
| ${PROJECT_SOURCE_DIR}/vocabulary/include | ||
| ${PROJECT_SOURCE_DIR}/utility/include | ||
| ${PROJECT_SOURCE_DIR}/primitives/include | ||
| ${PROJECT_SOURCE_DIR}/design/include | ||
| ${CMAKE_BINARY_DIR}/generated/iceoryx_hoofs/include | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have an additional layer additional indentation?
| BUILD_INTERFACE | |
| ${PROJECT_SOURCE_DIR}/include | |
| ${PROJECT_SOURCE_DIR}/legacy/include | |
| ${PROJECT_SOURCE_DIR}/memory/include | |
| ${PROJECT_SOURCE_DIR}/container/include | |
| ${PROJECT_SOURCE_DIR}/vocabulary/include | |
| ${PROJECT_SOURCE_DIR}/utility/include | |
| ${PROJECT_SOURCE_DIR}/primitives/include | |
| ${PROJECT_SOURCE_DIR}/design/include | |
| ${CMAKE_BINARY_DIR}/generated/iceoryx_hoofs/include | |
| BUILD_INTERFACE | |
| ${PROJECT_SOURCE_DIR}/include | |
| .... | |
| INSTALL_INTERFACE | |
| ... | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried my best to maintain the same form of indentation, but it seems that I do not have a control on the number of spaces/tabs between the columns. Please check the last commit
        
          
                iceoryx_hoofs/CMakeLists.txt
              
                Outdated
          
        
      | 
               | 
          ||
| # | ||
| ########## build iceoryx hoofs lib ########## | ||
| # ######### build iceoryx hoofs lib ########## | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the formatter interprets this as comment, maybe another separator symbol would be good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formatter forces a space between the comment mark and the being of the comment, I am disabling the format before this section as a solution,what do you think ?
Signed-off-by: Ziad Mostafa <[email protected]>
Signed-off-by: Ziad Mostafa <[email protected]>
Signed-off-by: Ziad Mostafa <[email protected]>
Signed-off-by: Ziad Mostafa <[email protected]>
001866c    to
    cc2ff14      
    Compare
  
    
          Codecov Report
 
 @@            Coverage Diff             @@
##           master    #1885      +/-   ##
==========================================
- Coverage   75.47%   74.97%   -0.51%     
==========================================
  Files         383      383              
  Lines       15166    14692     -474     
  Branches     2146     2099      -47     
==========================================
- Hits        11447    11015     -432     
+ Misses       3047     3031      -16     
+ Partials      672      646      -26     
 Flags with carried forward coverage won't be shown. Click here to find out more. 
  | 
    
| 
           @zmostafa are you planing to continue working on this PR?  | 
    
| 
           @elBoberido Yes, I am waiting for @dkroenke 's feedback for the new format, since applying cmake lint will change the current files format.  | 
    
Signed-off-by: Ziad Mostafa [email protected]
Pre-Review Checklist for the PR Author
iox-123-this-is-a-branch)iox-#123 commit text)task-list-completed)iceoryx_hoofsare added to./clang-tidy-diff-scans.txtNotes for Reviewer
@dkroenke I create this PR to address issue #1368 .
I am using cmakelang and have some open points stll:
tab sizeto 4 instead of 2, I think we need to revisit this config as applying them locally drastically changed the files.Checklist for the PR Reviewer
iceoryx_hoofshave been added to./clang-tidy-diff-scans.txtPost-review Checklist for the PR Author
References