Skip to content

Conversation

@datumbox
Copy link
Contributor

In Vision-2372 we fixed the clang-format version. @mthrok proposed to do the same in audio.

Copy link
Contributor Author

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Left some comments to clarify changes, please have a look.

install_build_dependencies() {
printf "* Installing torchaudio dependencies except PyTorch - (Python: %s)\n" "$1"
conda env update -q --file "${_this_dir}/environment.yml" --prune
if [ "${_os}" == Linux ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe clang-format is not actually used in the specific tests, so I removed it.

conda env update --file "${this_dir}/environment.yml" --prune
if [ "${os}" == Linux ] ; then
pip install clang-format
clangformat_path="${root_dir}/clang-format"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand other libraries like conda are put directly in root. So I do the same with clang-format.


printf "\x1b[34mRunning clang-format: "
clang-format --version
./clang-format --version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our root_dir is ./ here. Similar to what we do above with conda.

Copy link
Contributor

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@datumbox datumbox merged commit 04733c0 into pytorch:master Oct 22, 2020
@datumbox
Copy link
Contributor Author

The failed test are unrelated so we agreed with @mthrok to merge. Thanks for the quick review!

@datumbox datumbox deleted the ci/fix-clang-version branch October 22, 2020 17:29
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.

2 participants