BoostGSoC20 / ublas

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

Add & Test Clang Tidy Review Script #14

Closed coder3101 closed 4 years ago

github-actions[bot] commented 4 years ago

This Pull request Passed all of clang-tidy tests. :+1:

coder3101 commented 4 years ago

@bassoy, I have written the review script and now I am testing it here 🤞

You can see above that this PR passed all checks because this PR did not actually change any cpp/hpp file at that time.

coder3101 commented 4 years ago

Checking if the bot will spam when PR is synchronized! 🤞

coder3101 commented 4 years ago

Bot is spamming 😞

I will try to fix it tomorrow.

github-actions[bot] commented 4 years ago

This Pull request Passed all of clang-tidy tests. :+1:

github-actions[bot] commented 4 years ago

This Pull request Passed all of clang-tidy tests. :+1:

coder3101 commented 4 years ago

Okay, So everything seems fine by now! Let's see if bot only reviews the newly changed files or not. For simplicity, I will format a file with clang-format.

coder3101 commented 4 years ago

strides.hpp did not have any clang-tidy issues or warnings. I will format another file fixed_rank_stride.hpp which does have issues to check if they are reported by clang-tidy bot.

coder3101 commented 4 years ago

As this PR was synchronized, only the new changes have been reported by the Bot as you can see above.

Now, I will revert all the code formattings, It will have code just for Clang tidy review which @bassoy should review. I also request you to please approve the PR when you find it acceptable.

coder3101 commented 4 years ago

@bassoy Please review and approve the PR

bassoy commented 4 years ago

You have many checks queued. Can you cancel those which are not necessary?

coder3101 commented 4 years ago

You have many checks queued. Can you cancel those which are not necessary?

Sure

coder3101 commented 4 years ago

After this work is merged. I will take some time and try to implement ccache for caching our build artifacts for a faster build times.

bassoy commented 4 years ago

After this work is merged. I will take some time and try to implement ccache for caching our build artifacts for a faster build times.

Very good. However, I think our biggest concern with our CI-time is the queuing time.

coder3101 commented 4 years ago

After this work is merged. I will take some time and try to implement ccache for caching our build artifacts for a faster build times.

Very good. However, I think our biggest concern with our CI-time is the queuing time.

The Queue time is only for this PR, because it is from same repository, every push to this PR is being built twice (one for Push and Another for Pull), In General Push or Pull request from other contributors. Will not have this much tests.

There is another way to significantly decrease the Queue by 'fail-fast' strategy. If any one build test fails the complete queue is cancelled. Currently it is set to false, so even if clang format fails the jobs run on gcc/Linux etc.

We can turn fail fast to true, in future.

coder3101 commented 4 years ago

After I fixed the last issue the number of issues reported by clang-tidy decreased by one. This proved that we are indeed linting on our ublas repository, not on boostorg submodule clone of ublas.

@bassoy I request you to please review it now!

bassoy commented 4 years ago

You can merge the pull request to github-actions branch. please comment your python script before you do so.

coder3101 commented 4 years ago

I will do so and merge it tomorrow.