Skip to content

Add cpp-linter action #3

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

Merged
merged 2 commits into from
Jun 10, 2025
Merged

Add cpp-linter action #3

merged 2 commits into from
Jun 10, 2025

Conversation

Mousius
Copy link
Member

@Mousius Mousius commented Jun 9, 2025

This uses the same .clang-format as numpy for consistency.

@Mousius
Copy link
Member Author

Mousius commented Jun 9, 2025

@seiko2plus do you think this is good enough, or do you think we should set up spin to match the main repo?

@Mousius Mousius requested a review from Copilot June 9, 2025 21:36
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a C++ linting step to CI and ensures consistent formatting with NumPy’s .clang-format.

  • Introduces a GitHub Actions workflow (cpp-linter.yml) that runs a C++ linter on changes under npsr/.
  • Adds a .clang-format file mirroring NumPy’s style for uniform code formatting.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/cpp-linter.yml New CI workflow to run a C++ linter via the cpp-linter-action.
.clang-format Clang-Format configuration based on NumPy’s style guidelines.
Comments suppressed due to low confidence (1)

.clang-format:28

  • The regex for matching angled includes uses a closing quote (") instead of a closing angle bracket (>). It should be ^<[[:alnum:]_.]+> to properly match <header> patterns.
  - Regex:           '^<[[:alnum:]_.]+'"

style: "file"

- name: Linter Outputs
if: steps.linter.outputs.checks-failed != 0
Copy link
Preview

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

GitHub Actions outputs are treated as strings; comparing checks-failed to an unquoted number may always evaluate to true. Use != '0' or compare as a string to correctly detect failures.

Suggested change
if: steps.linter.outputs.checks-failed != 0
if: steps.linter.outputs.checks-failed != '0'

Copilot uses AI. Check for mistakes.

This uses the same .clang-format as numpy for consistency.
@seiko2plus
Copy link
Member

I followed google style without any modifications in #1 to match with Highway, but its okay to reformat it.. I just followed Highway because I thought that's maybe its better to target Highway community in case of there's need to move/share any code..

@seiko2plus
Copy link
Member

do you think this is good enough, or do you think we should set up spin to match the main repo?

For now its fine to me.. I just add it spin within #1, with support of spin commands (test, generate, python, ipython)

@seiko2plus seiko2plus merged commit 62f49dc into numpy:main Jun 10, 2025
@seiko2plus
Copy link
Member

@Mousius, Thank you!

@Mousius Mousius deleted the clang-tidy branch June 10, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants