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

Lint with clang tidy #73

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Conversation

Lai-YT
Copy link
Collaborator

@Lai-YT Lai-YT commented Feb 22, 2024

What's changed?

Incorporated the static analyzer "clang-tidy" into our project to improve code quality.

  • Added a make target tidy to perform tidy checks on the entire project. The compilation database compile_commands.json is not required for this target as the Makefile already contains the necessary commands.
  • Introduced a CI job to conduct tidy checks on the difference between main (GITHUB_BASE_REF) and HEAD. We utilize Bear to generate the compilation database.

Additionally, a script is added for CI execution. This script is not intended for local user execution due to the following reasons:

  1. clang-tidy-diff supports exit with a failure code when there are errors from version 17 or 18 onward. (The exact version is uncertain. For more details, please refer to clang-tidy-diff.py should exit with a failure code when there are errors llvm/llvm-project#65000.)
  2. bear has different command line interfaces between version <= 2.4.x and onward. (For more details, please refer to Bear's documentation.)

The script may not function properly if any of these issues arise. In the CI environment, we ensure that the versions of these dependencies are up-to-date. Specifically, the version of clang-tidy-diff is specified as 18, and the version of Bear on Ubuntu 22.04 is 3.0.x.

How to test?

I pushed a testing commit which violates the naming rule to fail the CI, and it did fail as expected.

Resolves: #71

@Lai-YT Lai-YT added the ci label Feb 22, 2024
@Lai-YT Lai-YT requested a review from leewei05 February 22, 2024 15:49
@Lai-YT Lai-YT self-assigned this Feb 22, 2024
@Lai-YT Lai-YT force-pushed the lint-with-clang-tidy branch 25 times, most recently from 31e4cd2 to b4c64c2 Compare February 23, 2024 02:44
Added a script for CI execution. This script is not intended for local
user execution due to the following reasons:

1. `clang-tidy-diff` supports exit with a failure code when there are
   errors from version 17 or 18 onward. (The exact version is uncertain.
   For more details, please refer to
   llvm/llvm-project#65000.)
2. `bear` has different command line interfaces between version <= 2.4.x
    and onward. (For more details, please refer to
    https://github.com/rizsotto/Bear?tab=readme-ov-file#how-to-use.)

The script may not function properly if any of these issues arise. In
the CI environment, we ensure that the versions of these dependencies
are up-to-date. Specifically, the version of `clang-tidy-diff` is
specified as `18`, and the version of `Bear` on Ubuntu 22.04 is 3.0.x.
These jobs check different aspects of the code, and one should not
depend on the other to succeed.
Copy link
Contributor

@leewei05 leewei05 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@leewei05 leewei05 merged commit a79ed0d into fruits-lab:main Feb 23, 2024
4 checks passed
@Lai-YT Lai-YT deleted the lint-with-clang-tidy branch February 23, 2024 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint with clang-tidy
2 participants