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.

Sign up for GitHub

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 clang format version #1755

Merged
merged 2 commits into from
May 24, 2021
Merged

add clang format version #1755

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59c23eb
add clang format version
danewalton-msft May 17, 2021
8735625
Merge branch 'master' into dawalton/clangformatversion
danewalton-msft May 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eng/pipelines/templates/jobs/archetype-sdk-client.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ jobs:
# Validate all the files are formatted correctly according to the .clang-format file.
- bash: |
# Run clang-format recursively on each source and header file within the repo.
clang-format --version
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea. Out of curiosity, why did you notice you needed it? Did you observe the version mismatch between CI and what you had locally causing formatting difference?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like CI currently has it as:
clang-format version 9.0.0-2~ubuntu18.04.2 (tags/RELEASE_900/final)

Copy link
Contributor

@ahsonkhan ahsonkhan Jul 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For C++, we stated we want v10 (and moved the test to a MacOS leg), so that our CI machines use the slightly newer version:
https://github.com/Azure/azure-sdk-for-cpp/blob/main/CONTRIBUTING.md#third-party-dependencies
Azure/azure-sdk-for-cpp#1328

We seem to be OK with v9 here, which is totally fine, it might be worth mentioning the version to contributors though, here:
https://github.com/Azure/azure-sdk-for-c/blob/main/CONTRIBUTING.md#prerequisites

Copy link
Contributor

Choose a reason for hiding this comment

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

Found the issue that motivated us making the change in C++:
Azure/azure-sdk-for-cpp#1207

@vhvb1989 what are your thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

iirc it was because different versions create differently rendered files. this was just to make sure people could easily see which version is being used at the gates

find . \( -iname '*.h' -o -iname '*.c' \) -exec clang-format -i {} \;

git status --untracked-files=no --porcelain
Expand Down