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

Add lint to CI include checking for cmake-format, clang-format and license header #30

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

chaojun-zhang
Copy link
Contributor

@chaojun-zhang chaojun-zhang commented Mar 3, 2023

Add lint to CI include checking for cmake-format, clang-format and license header, see #15

Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on. Some initial thoughts.

.github/workflows/build_test.yml Outdated Show resolved Hide resolved
.github/workflows/build_test.yml Outdated Show resolved Hide resolved
.github/workflows/build_test.yml Outdated Show resolved Hide resolved
@chaojun-zhang
Copy link
Contributor Author

Hi @westonpace , please help to re-run and trigger the check again.

Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Some small suggestions and then I think this is good to go.

scripts/run-clang-tidy.py Outdated Show resolved Hide resolved
.github/workflows/build_test.yml Outdated Show resolved Hide resolved
scripts/run-clang-tidy.py Outdated Show resolved Hide resolved
@westonpace
Copy link
Member

I'm not sure why the build is not running. Perhaps you need to rebase to get the latest github workflows?

@mbrobbel
Copy link
Member

I'm not sure why the build is not running. Perhaps you need to rebase to get the latest github workflows?

https://github.com/substrait-io/substrait-cpp/actions/runs/4459109898/workflow

@chaojun-zhang
Copy link
Contributor Author

chaojun-zhang commented Mar 21, 2023

check failed due to file run-clang-tidy.py is not exist in current project , so PR #37 should be merge first.

westonpace pushed a commit that referenced this pull request Mar 22, 2023
This PR should be the first PR of issue
#15, and then the
second [PR](#30) will
use this for lint check.

- please see https://github.com/apache/skywalking-eyes for license check
- run-clang-tidy.py : a python script used to check cpp project with
clang-tidy

Co-authored-by: chaojun-zhang <zcj23085@gmail.com>
@westonpace
Copy link
Member

I've merged #37. Do you want to rebase now? If it's still failing or we need changes to those files that are checked in then let me know and I should have some time to work it out on Friday too.

@chaojun-zhang
Copy link
Contributor Author

Rebased and finally passed

@chaojun-zhang chaojun-zhang requested review from EpsilonPrime and westonpace and removed request for westonpace and EpsilonPrime March 23, 2023 07:01
@EpsilonPrime
Copy link
Member

I'm looking forward to this one, it'll be nice to have something to catch when I forget the license headers!

Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Thanks for this help!

@westonpace westonpace merged commit 4153a00 into substrait-io:main Mar 23, 2023
EpsilonPrime pushed a commit to EpsilonPrime/substrait-cpp that referenced this pull request Mar 24, 2023
This PR should be the first PR of issue
substrait-io#15, and then the
second [PR](substrait-io#30) will
use this for lint check.

- please see https://github.com/apache/skywalking-eyes for license check
- run-clang-tidy.py : a python script used to check cpp project with
clang-tidy

Co-authored-by: chaojun-zhang <zcj23085@gmail.com>
EpsilonPrime pushed a commit to EpsilonPrime/substrait-cpp that referenced this pull request Mar 24, 2023
EpsilonPrime pushed a commit to EpsilonPrime/substrait-cpp that referenced this pull request May 6, 2023
This PR should be the first PR of issue
substrait-io#15, and then the
second [PR](substrait-io#30) will
use this for lint check.

- please see https://github.com/apache/skywalking-eyes for license check
- run-clang-tidy.py : a python script used to check cpp project with
clang-tidy

Co-authored-by: chaojun-zhang <zcj23085@gmail.com>
EpsilonPrime pushed a commit to EpsilonPrime/substrait-cpp that referenced this pull request May 6, 2023
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.

4 participants