roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

add scons fmt check for travis linux build #342

Closed fusuiyi123 closed 4 years ago

fusuiyi123 commented 4 years ago

Fixes #309

gavv commented 4 years ago

Cool!

Please re-target PR to develop branch, as stated in CONTRIBUTING.md.

Also, let's move the check (scons + git) to a script, say, scripts/travis/linux-build-checks/code-formatting.sh.

Also, it would be nice to produce a meaningful error message if the check fails. Something like: "the code is not formatted, please run scons fmt, using clang-format version x.y.z". And probably to would be useful to print a diff to see what exactly is not formatted.

And we should run the script inside a container with scons and clang-format installed. You can find the list of available images, and documentation on them, here. I think we can use travis-checks image. If it doesn't have all necessary tools, you can add them by issuing a pull request to this repo. After merging the PR, Docker Hub will automatically rebuild the image from the updated Dockerfile.

What clang-format version to you use? I'm using 9.0.1 and it does not modify of_decoder.cpp when I run scons fmt.

fusuiyi123 commented 4 years ago

thanks! this is my version, clang-format version 9.0.0 (tags/google/stable/2019-05-14) let me rebase to development. Actually I tried moving scons and git to script earlier but it will give git command not found error, I will try and search again.

fusuiyi123 commented 4 years ago

https://github.com/roc-project/dockerfiles/pull/2

gavv commented 4 years ago

thanks! this is my version, clang-format version 9.0.0 (tags/google/stable/2019-05-14) let me rebase to development

Ah, got it, of_decoder.cpp was modified because it was not formatted in master. OK.

fusuiyi123 commented 4 years ago

cool, applied your comments

gavv commented 4 years ago

We should also add docker pull rocproject/travis-checks to before_install section of Linux build checks job.

gavv commented 4 years ago

dockerfiles PR is merged. Docker hub will rebuild the image in a few hours. https://hub.docker.com/r/rocproject/travis-checks

UPD: the image was updated as well

gavv commented 4 years ago

Thanks!