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

Run a CI job linting/parsing the CMake files in cmake/* #222

Draft
wants to merge 14 commits into
base: ign-cmake2
Choose a base branch
from

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Apr 1, 2022

🎉 New feature

Summary

There is no current check on very broken CMake syntax in our cmake modules (see #221 as an example). Looking for some tools I found cmakelang that provides a parser and linter that was able to find parsing problems on the error present in #221.

The tool is not perfect and sometimes just crash on the presence of errors but serves well as smoke testing. It also brings all kind of conventions that is some nice to have and define in the ign-cmake project.

The PR consists in two parts:

  • Setup a github action and a cmakelang configuration file to enable CI on the cmake/* files.
  • Apply all kind of fixes to current files that do not alter the behavior or API provided.

Still a Draft until I finish with other PRs affecting OGRE2. We need to include documentation about what conventions we want to follow.

Test it

The github action should take care of test the whole setup.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel labels Apr 1, 2022
@chapulina chapulina added enhancement New feature or request and removed 🌱 garden Ignition Garden labels Jul 25, 2022
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. What are next steps to getting in landed? Addressing the remaining outstanding issues?

@j-rivero
Copy link
Contributor Author

This looks great. What are next steps to getting in landed? Addressing the remaining outstanding issues?

I think so yes.

@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
@azeey azeey removed the beta Targeting beta release of upcoming collection label Aug 18, 2023
@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 29, 2024
@azeey azeey removed the beta Targeting beta release of upcoming collection label Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel enhancement New feature or request 🏯 fortress Ignition Fortress Gazebo 1️1️ Dependency of Gazebo classic version 11
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

4 participants