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

Static analysis in CI #42

Open
friendlyanon opened this issue Oct 9, 2023 · 3 comments
Open

Static analysis in CI #42

friendlyanon opened this issue Oct 9, 2023 · 3 comments

Comments

@friendlyanon
Copy link
Contributor

friendlyanon commented Oct 9, 2023

The contributing document suggests that contributors ought to format the code before submission, however there is no enforcement in CI. I would like to add it to the CI, however it's not stated which version of clang-format should be used. Different versions of clang-format may format some code differently, so it's important to specify the version.

On the same note, I'd like to also know your opinions on other forms of static analysis, such as spell checking. I have been using codespell personally. If you find it satisfactory, I'd also like to add that in the same PR.

To make efficient use of CI resources, I would like to structure CI as a single file with such job and matrix configuration:

flowchart
    A["clang-format
    codespell"]
    B["CMake (Windows)
    CMake (Ubuntu)"]
    C["CodeQL (Windows)
    CodeQL (Ubuntu)"]
    A --> B
    A --> C
Loading

You can see this example, where nothing else is done until the lint job is done.

Making this chart also makes me question whether the separate CMake jobs are necessary with CodeQL jobs being a thing.

@GabrielDosReis
Copy link
Collaborator

Thanks for writing this up. The team is still discussing it. Given resources constraints, CodeQL will take precedence over spell checking and clang formatting. As you correctly observe, clang-formatting really is version-dependent and we are reluctant to cause or require churns just for that. We are relying on ClangFormat primarily as a way to reduce review traffics that are solely formatting related. For now, there is hesitancy to make it a check-in gate, unless we start seeing deviations in submitted PRs that go beyond reasonable.

@friendlyanon
Copy link
Contributor Author

Understandable. Would it be an issue even if the diagnostics were just informative? clang-format and codespell run pretty fast when using an Ubuntu runner.

How about combining the existing cmake.yml with codeql.yml? At present, the overlap in their functionality is significant.

@GabrielDosReis
Copy link
Collaborator

How about combining the existing cmake.yml with codeql.yml? At present, the overlap in their functionality is significant.

Whatever is done, we need the CodeQL workflow as a separate workflow because it is needed by organizational policies.
So, even if cmake.yml and codeql.yml is combined, we would still need codeql.yml; so that leads to some form of duplication.

Could you tell me more about the "optimization" goal here and what are the alternatives available?

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

No branches or pull requests

2 participants