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] Don't diff when running clang-format #10933

Merged
merged 4 commits into from
Apr 11, 2022
Merged

[ci] Don't diff when running clang-format #10933

merged 4 commits into from
Apr 11, 2022

Conversation

driazati
Copy link
Member

@driazati driazati commented Apr 7, 2022

This takes about 15-20 extra seconds but has the benefit of allowing users to replicate and fix clang format issues locally with ease. This also adds a --fix flag to ci.py that can be used to run the Docker image clang-format/black in-place on the mounted repo:

python tests/scripts/ci.py lint --fix

The second commit in this PR contains all the formatting fix-ups necessary.

cc @areusch

This takes about 15-20 extra seconds but has the benefit of allowing users to replicate and fix clang format issues locally with ease.
@driazati driazati changed the title clang [ci] Don't diff when running clang-format Apr 7, 2022
@shingjan
Copy link
Contributor

shingjan commented Apr 7, 2022

Thanks for sending out this PR! @driazati Does this solve the similar problem with #10895 but on the cpp side?

@driazati
Copy link
Member Author

driazati commented Apr 7, 2022

Right this should fix the issue you had earlier regarding the difference in output between CI / local

@github-actions github-actions bot requested a review from areusch April 7, 2022 22:44
# check lastest change, for squash merge into main
./tests/lint/git-clang-format.sh HEAD~1
# chekc against origin/main for PRs.
./tests/lint/git-clang-format.sh origin/main
Copy link
Contributor

Choose a reason for hiding this comment

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

confused, shouldn't this have caught all of these mis-formats?

Copy link
Member Author

Choose a reason for hiding this comment

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

it only ran against the diff, these must have snuck in at some point and then they would only come up when those file names were changed

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, this makes more sense. reviewed more carefully now

tests/lint/git-clang-format.sh Outdated Show resolved Hide resolved
@areusch areusch merged commit 396a8c6 into apache:main Apr 11, 2022
Lucien0 pushed a commit to Lucien0/tvm that referenced this pull request Apr 19, 2022
* [ci] Don't diff when running clang-format

This takes about 15-20 extra seconds but has the benefit of allowing users to replicate and fix clang format issues locally with ease.

* format files

* Add --fix flag

* Comments

Co-authored-by: driazati <driazati@users.noreply.github.com>
altanh pushed a commit to altanh/tvm that referenced this pull request Apr 28, 2022
* [ci] Don't diff when running clang-format

This takes about 15-20 extra seconds but has the benefit of allowing users to replicate and fix clang format issues locally with ease.

* format files

* Add --fix flag

* Comments

Co-authored-by: driazati <driazati@users.noreply.github.com>
areusch added a commit to areusch/tvm that referenced this pull request Jun 2, 2022
jroesch pushed a commit that referenced this pull request Jun 2, 2022
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.

3 participants