Skip to content
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

Fix cpplint test in motion_computation to support CI/CD #2405

Merged
merged 7 commits into from
Jun 14, 2024

Conversation

MishkaMN
Copy link
Contributor

@MishkaMN MishkaMN commented Jun 11, 2024

PR Details

Description

Fixing motion_computation cpplint errors introduced in here:
#2399
with the failing github action:
https://github.com/usdot-fhwa-stol/carma-platform/actions/runs/9449278789/job/26037602316?pr=2397#step:13:23837

My environment was never configured for clang format and only used to delete whitespaces. I improved it by:

  1. I have installed clang by sudo apt install clang
  2. installed cpplint by sudo pip install cpplint
  3. I have then installed vscode extensions that use packages Clang-Format and cpplint
  4. created a .clang-format file outside the src folder that has all the repos and started editing to get the format. If there is multiple, I believe the immediate .clang-format file in the project gets to be used first.
  5. added following to my user settings.json in vscode (F1 -> "Preferences: Open user settings.json")
    "[cpp]": {
        "editor.defaultFormatter": "xaver.clang-format"
    },
    "editor.wordWrap": "wordWrapColumn",
    "editor.wrappingIndent": "indent",
    "editor.wordWrapColumn": 100,
    "editor.formatOnSave": true,
    "editor.rulers" :[
        100
    ],
    "C_Cpp.clang_format_fallbackStyle": "Google",
    "cpplint.lineLength": 100

For a lot of the things to take effect and add vertical line indicating the 100 line limit, which one can make false

The cpplint in the CICD is still complaining about the order of the headers. After trying multiple orders, I found an order in autoware.auto that does the job. Despite visually it is correct, not sure why cpplint is still complaining (which carma_cooperative_perception also seems to have encountered). Therefore, turning off the check for include orders for now as we haven't established strict convention with it anyways.

Related GitHub Issue

NA

Related Jira Key

NA

Motivation and Context

NA

How Has This Been Tested?

CICD passes

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@MishkaMN MishkaMN requested a review from JonSmet June 11, 2024 17:26
@MishkaMN MishkaMN changed the title fix white space Fix white space in motion_computation to support CI/CD Jun 11, 2024
@MishkaMN MishkaMN self-assigned this Jun 11, 2024
@MishkaMN MishkaMN added the anomaly Something isn't working label Jun 11, 2024
@MishkaMN
Copy link
Contributor Author

Looks like motion_computation doesn't have cpp lint failures anymore. But approaching_emergency_vehicle_plugin and gnss_to_map_convertor have intermittent test failures due to timeouts. Possibly related to: #2335
Github Action to track
https://github.com/usdot-fhwa-stol/carma-platform/actions/runs/9490590550/job/26160807595?pr=2405

@MishkaMN MishkaMN changed the title Fix white space in motion_computation to support CI/CD Fix cpplint test in motion_computation to support CI/CD Jun 13, 2024
@MishkaMN MishkaMN requested a review from JonSmet June 13, 2024 03:51
@MishkaMN MishkaMN requested a review from JonSmet June 14, 2024 15:13
@MishkaMN MishkaMN requested review from JonSmet and removed request for JonSmet June 14, 2024 16:33
Copy link

sonarcloud bot commented Jun 14, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
16.67% Line Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@MishkaMN MishkaMN merged commit 2fee8dc into develop Jun 14, 2024
3 of 4 checks passed
@MishkaMN MishkaMN deleted the fix/motion-computation-white-space branch June 14, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
anomaly Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants