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] Add cpp file format checker #2425

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

DonghakPark
Copy link
Member

This patch adds a Github Action workflow to check cpp file format

  • this file imported from deviceMLOps.MLAgent
  • this file use cpp_linter marketplace actions

Self evaluation:

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

@taos-ci
Copy link
Collaborator

taos-ci commented Jan 24, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2425. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

@taos-ci
Copy link
Collaborator

taos-ci commented Jan 24, 2024

:octocat: cibot: @DonghakPark, The last line of a text file must have a newline character. Please append a new line at the end of the line in .github/workflows/cpp_linter.yml.

This patch adds a Github Action workflow to check cpp file format
- this file imported from deviceMLOps.MLAgent
- this file use cpp_linter marketplace actions

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

Signed-off-by: Donghak PARK <[email protected]>
Copy link
Collaborator

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

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

@DonghakPark, 💯 All CI checkers are successfully verified. Thanks.

Copy link
Contributor

@djeong20 djeong20 left a comment

Choose a reason for hiding this comment

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

quick question. current TAOS/pr-prebuild-clang specify the exact line to fix. would this work the same?

@DonghakPark
Copy link
Member Author

quick question. current TAOS/pr-prebuild-clang specify the exact line to fix. would this work the same?

This action based on marketplace gitaction : https://github.com/marketplace/actions/c-c-linter
And according to explain they give line by line feedback !

Copy link
Member

@SeoHyungjun SeoHyungjun left a comment

Choose a reason for hiding this comment

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

As far as I know, the official extension is yaml.
I understand that yml was created because the extension is limited to 3 characters in Windows.
How about changing it to yaml?

@DonghakPark
Copy link
Member Author

As far as I know, the official extension is yaml. I understand that yml was created because the extension is limited to 3 characters in Windows. How about changing it to yaml?

I totally agree with you . The three-character limit is no longer a meaningful rule.
However, since the default value for adding a new action in GitHub is defined as YML, all the GitAction files in NNTrafter have been unified to YML. I believe it is for wider compatibility and GitHub uses YML as the default value for GitAction.

What do you think about conforming to the default settings on Github or is it better to change official express ?

Copy link
Collaborator

@jijoongmoon jijoongmoon left a comment

Choose a reason for hiding this comment

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

LGTM

@jijoongmoon jijoongmoon merged commit 4a7f3c2 into nnstreamer:main Jan 26, 2024
30 checks passed
@DonghakPark DonghakPark deleted the gitaction_clang branch May 7, 2024 00:44
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.

5 participants