Stick clang-format version to 11 and add back docker-format.sh#45
Conversation
BewareMyPower
added
documentation
BewareMyPower
requested review from
Demogorgon314,
RobertIndie,
merlimat and
michaeljmarshall
|
When installing clang-format on current macos & ubuntu, the default is now 11.0. We don't need to worry about the |
|
@merlimat Yeah, removing these scripts looks good to me too.
So this PR would aim at sticking the clang-format to 11.0.0. |
BewareMyPower
force-pushed
the
bewaremypower/fix-docker-build
branch
from
de01549 to
239473e
Compare
BewareMyPower
changed the title
BewareMyPower
force-pushed
the
bewaremypower/fix-docker-build
branch
from
239473e to
d7cc680
Compare
|
I have updated this PR, PTAL again @merlimat |
RobertIndie
left a comment
RobertIndie
left a comment
There was a problem hiding this comment.
Yeah, removing these scripts looks good to me too.
According to the discussion, seems we will not worry about the docker-format.sh. But this file still be added in this PR. Is there some context I have missed?
|
@RobertIndie The previous Though this PR adds |
| @@ -0,0 +1,35 @@ | |||
| # | |||
There was a problem hiding this comment.
Maybe we could move this into build-support. That would also help to not recreate the image each time any cpp file is changed.
There was a problem hiding this comment.
Moved. But I don't get why will the image be recreated each time any cpp file is changed.
|
|
||
| For Ubuntu users, you can install `clang-format-11` via `apt install clang-format-11`. For other users, run `./build-support/docker-format.sh` if you have Docker installed. | ||
|
|
||
| We welcome contributions from the open source community, kindly make sure your changes are backward compatible with GCC 4.8 and Boost 1.53. |
There was a problem hiding this comment.
Do we have requirements on the version number of Boost? If yes, it needs to be shown in the Requirements section.
There was a problem hiding this comment.
The dependency compatibility is hard to test. line 229 is added in a very early time.
| ## Requirements for Contributors | ||
|
|
||
| It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files. `make format` automatically formats the files. | ||
| It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` **11** to format files. `make format` automatically formats the files. |
There was a problem hiding this comment.
| It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` **11** to format files. `make format` automatically formats the files. | |
| It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ clients use `clang-format` **11** to format files. `make format` automatically formats the files. |
There was a problem hiding this comment.
Thanks for your suggestions, I will address them later after since the CI is very unstable now.
### Motivation Currently the code is formatted by `clang-format` 11. If it's formatted by other versions of `clang-format`, the style might be a little different. ### Modifications - Add back `docker-format.sh` and the associated `Dockerfile.format`. This script build the image with `python3` and `clang-format-11` installed and the entrypoint is formatting the code. - Explain the `clang-format` version in README.
BewareMyPower
force-pushed
the
bewaremypower/fix-docker-build
branch
from
68155b8 to
e17157f
Compare
* Stick clang-format version to 11 and add back docker-format.sh ### Motivation Currently the code is formatted by `clang-format` 11. If it's formatted by other versions of `clang-format`, the style might be a little different. ### Modifications - Add back `docker-format.sh` and the associated `Dockerfile.format`. This script build the image with `python3` and `clang-format-11` installed and the entrypoint is formatting the code. - Explain the `clang-format` version in README. * Move Dockerfile and docker-format.sh to build-support (cherry picked from commit ecc1995)
5 participants
Motivation
Currently the code is formatted by
clang-format11. If it's formattedby other versions of
clang-format, the style might be a littledifferent.
Modifications
docker-format.shand the associatedDockerfile.format.This script build the image with
python3andclang-format-11installed and the entrypoint is formatting the code.
clang-formatversion in README.Documentation
doc-required(Your PR needs to update docs and you will update later)
doc-not-needed(Please explain why)
doc(Your PR contains doc changes)
doc-complete(Docs have been already added)