Skip to content

Conversation

rugeGerritsen
Copy link
Contributor

@rugeGerritsen rugeGerritsen commented Oct 1, 2025

It is very common to see a comment stating that the "No Space Before Opening Brackets" style guide is not followed. To avoid putting this burdon on reviewers, let the compliance check catch this instead.

Checking for tab indentation was also very simple, so also added this.

Running this check on all files finds many violations:

  • Space before opening brackets: 141 violations
  • Tab indentation: 420 violations

Fixing these will not be done as part of this PR. Adding the check will prevent us from adding new violations.

Attached file with list of current style violations:
CMakeStyle.txt

carlescufi
carlescufi previously approved these changes Oct 1, 2025
Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me, thanks!

It is very common to see a comment stating that the
"No Space Before Opening Brackets" style guide is not followed.
To avoid putting this burdon on reviewers, let the compliance
check catch this instead.

Checking for tab indentation was also very simple, so also added
this.

Running this check on all files finds many violations:
 - Space before opening brackets: 141 violations
 - Tab indentation: 420 violations

Fixing these will not be done as part of this PR. Adding the check
will prevent us from adding new violations.

Signed-off-by: Rubin Gerritsen <[email protected]>
Copy link

sonarqubecloud bot commented Oct 1, 2025

mime=True) == "text/x-python")]


class CMakeStyle(ComplianceTest):
Copy link
Member

Choose a reason for hiding this comment

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

IMO it is better to split this into own script to be able to extend it in the future with more cmake style checks and also be able to run it standalone and just call that script from check_compliance.

Copy link
Contributor

Choose a reason for hiding this comment

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

The check is very rudimental, it doesn't check for elseif/else/endif/foreach/endforeach and probably many more.

Copy link
Member

Choose a reason for hiding this comment

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

yes, thart is my point. my concern is that we will start expanding the linter functionality inside check_compliance, which is in general a wrapper on top of linters, not a linter itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please let me know if you want me to update the commit. I would be happy if you could point me to existing examples of such linters and where to put the python script linting cmake code.

@nashif nashif merged commit e9abaf4 into zephyrproject-rtos:main Oct 5, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants