BoostGSoC20 / ublas

Boost.uBlas
https://www.boost.org/doc/libs/release/libs/numeric/ublas
1 stars 2 forks source link

Let CI check and correct code formatting #5

Closed coder3101 closed 4 years ago

coder3101 commented 4 years ago

Code style is a very important tool for an open-source project as it makes the source code consistent and more readable. As of now, there is no agreed-upon style for uBLAS. In this work, I do not propose a new code style but suggest we use clang-format for the formatting.

Check Code formatting upon git push, pullrequest ~Auto PR to fix it~

git push starts the CI to check for compilation and now it will also check for code formatting using the .clang-format file in the root of the Boost.uBLAS repository. If the code format does not confer to the style, the CI will fail. In the case of a Pull Request, the failed CI will not allow PR to be merged.

~I also wish to add an auto PR bot which will create a Pull Request in the Boost.uBLAS repository with the fixed code style, and in case of a Pull request that does not confer to the style, the bot will create a PR to the PR's author's branch with the fix, this way new contributors even without installing clang-format can propose correctly formatted code.~

EDIT: As per request from Cem, the Auto PR to fix which used peter-evans/create-pull-request actions, has been removed. Now, if format checking failed, the CI will fail and the user has to manually fix and again push a commit.

coder3101 commented 4 years ago

@bassoy Can we agree upon some code style? Do we need to only format code of tensor extension or complete Boost.ublas?

coder3101 commented 4 years ago

I am using the default code format style of clang-format. While merging this PR into boostorg/ublas we can agree to reach on some style.

The following codes are only affected by code formatting:

If you want to cover more areas with code formatting, please add to this line the location (you can use wildcards)

bassoy commented 4 years ago

@bassoy Can we agree upon some code style? Do we need to only format code of tensor extension or complete Boost.ublas?

@coder3101 Yes we need to discuss. Please provide an initial clang-format file so that we can discuss about it.

bassoy commented 4 years ago

As discussed in gitter, let us define not use third-party tools.

coder3101 commented 4 years ago

https://gist.github.com/andrewseidl/8066c18e97c40086c183

This gist compare different formatting that clang format by default offers. It is Google, LLVM, Chromium, Webkit, Microsoft and Mozilla. Each style is predefined and follows the guidelines set by that organization for C++ standards. If you like none of them, or feel any of above standard is good but need some change, it can be done as well.

You can check and see which style is best for ublas for starting.

bassoy commented 4 years ago

Let's start with rudimentary .clang_format file The explanation of all options are explained in https://clang.llvm.org/docs/ClangFormatStyleOptions.html I have concrete idea for most of the options. So once you have it, I would place a list in gitter or in a gist so we can discuss all together in gitter.

coder3101 commented 4 years ago

Here is the clang-format file that is based on Microsoft C++ Standards, You can tweak it based on your and uBLAS liking. You have already mentioned above the ClangFormatStyleOptions that can be used to customize the style.

Please make sure that you use the documentation for clang-format-9

coder3101 commented 4 years ago

This should be closed. I am tagging it done!

bassoy commented 4 years ago

Please make sure that you use the documentation for clang-format-9

why specifically 9.0.0 ?

coder3101 commented 4 years ago

Please make sure that you use the documentation for clang-format-9

why specifically 9.0.0 ?

We can use any version but the newser we use the more customisation of style can be done. Each version has its own separate documentation. But using too latest (10.0.0) can be problem as it will not be easily available on some LTS or debian without PPA. So, I have chosen a fairly modern and widely and easily available clang-format.

Plus config file for clang format is forward compatible. So a file written for 9.0.0 will work with any clang format > 9.0.0

bassoy commented 4 years ago

Plus config file for clang format is forward compatible. So a file written for 9.0.0 will work with any clang format > 9.0.0

I am okay with clang format 9.0.0.

bassoy commented 4 years ago

Let me go over the options. I will look at your proposed format and perhaps modify.

bassoy commented 4 years ago

Updated .clang-format