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

[CI] update static checker script to downgrade clang-format version. #2878

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

djeong20
Copy link
Contributor

This pull request updates the workflow script to run on Ubuntu 22.04 instead of the latest version, allowing the static checker to use clang-format version 14.

Self-evaluation:

  1. Build test: [ ]Passed [ ]Failed [X]Skipped
  2. Run test: [ ]Passed [ ]Failed [X]Skipped

Copy link
Member

@DonghakPark DonghakPark left a comment

Choose a reason for hiding this comment

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

Nice Catch!
And i think we can remove cpp_checker in static_check to avoid confusion .github/workflows/cpp_linter.yml seems to be working properly, except minor differences.

Copy link
Member

@skykongkong8 skykongkong8 left a comment

Choose a reason for hiding this comment

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

I suffered from this SO LONG!
Great work!

Copy link
Contributor

@EunjuYang EunjuYang left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@djeong20
Copy link
Contributor Author

Nice Catch! And i think we can remove cpp_checker in static_check to avoid confusion .github/workflows/cpp_linter.yml seems to be working properly, except minor differences.

Thank you for your feedback! I have removed the clang-format checks from the static checkers and verifiers. Please take a look.

@@ -23,6 +23,7 @@ jobs:
file-annotations: false
step-summary: true
format-review: true
tidy-checks: '-*' # disable clang-tidy checks.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disabling clang-tidy checks because the reports generated are just simple warnings and may provide incorrect results.

@DonghakPark
Copy link
Member

Nice Catch! And i think we can remove cpp_checker in static_check to avoid confusion .github/workflows/cpp_linter.yml seems to be working properly, except minor differences.

Thank you for your feedback! I have removed the clang-format checks from the static checkers and verifiers. Please take a look.

Thank you for quick apply!! LGTM again

Copy link
Contributor

@baek2sm baek2sm left a comment

Choose a reason for hiding this comment

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

LGTM

This pull request updates the workflow script to run on Ubuntu 22.04 instead of the latest version, allowing the static checker to use clang-format version 14.

**Self-evaluation:**
1. Build test: [ ]Passed [ ]Failed [X]Skipped
2. Run test:   [ ]Passed [ ]Failed [X]Skipped

Signed-off-by: Donghyeon Jeong <[email protected]>
This commit aims to eliminate duplicate clang-format checks, which can lead to mismatches.
Additionally, it updates the cpp-linter workflow script to run on Ubuntu 22.04 instead of the latest version.

**Changes proposed in this PR:**
- Remove the C++ checker from the static checker.
- Update cpp-linter to operate on Ubuntu 22.04 and disable clang-tidy checks.

**Self-evaluation:**
1. Build test: [ ]Passed [ ]Failed [X]Skipped
2. Run test:   [ ]Passed [ ]Failed [X]Skipped

Signed-off-by: Donghyeon Jeong <[email protected]>
@djeong20 djeong20 force-pushed the ci/static_checks/ubuntu_version branch from 883a283 to bd98485 Compare January 16, 2025 23:57
@jijoongmoon jijoongmoon merged commit c603787 into nnstreamer:main Jan 17, 2025
17 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.

6 participants