Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Jenkins] clang-tidy checks #2377

Merged
merged 12 commits into from Feb 27, 2019

Conversation

Projects
None yet
3 participants
@bilke
Copy link
Member

bilke commented Feb 22, 2019

Introduces new sub-job Clang-Analyzer. Set unstable threshold to current number of clang-tidy issues (58).

@endJunction Had to update clang to version 7 (and the base image of the Docker container to Ubuntu 18.04) to fix linker errors.

@bilke bilke force-pushed the bilke:clang-analyzer branch from e5f0769 to 2cb2fd2 Feb 22, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 23, 2019

Codecov Report

Merging #2377 into master will decrease coverage by 0.09%.
The diff coverage is 23.07%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2377     +/-   ##
========================================
- Coverage   32.69%   32.6%   -0.1%     
========================================
  Files         528     528             
  Lines       19847   19957    +110     
  Branches     9306    9369     +63     
========================================
+ Hits         6489    6507     +18     
- Misses      10049   10100     +51     
- Partials     3309    3350     +41
Impacted Files Coverage Δ
MeshGeoToolsLib/BoundaryElementsSearcher.cpp 41.46% <ø> (ø) ⬆️
MeshLib/ElementCoordinatesMappingLocal.h 100% <ø> (ø) ⬆️
MathLib/LinAlg/RowColumnIndices.h 100% <ø> (ø) ⬆️
MaterialLib/Adsorption/DensityLegacy.h 0% <ø> (ø) ⬆️
BaseLib/TimeInterval.h 0% <ø> (ø) ⬆️
.../RelativePermeability/WettingPhaseVanGenuchten.cpp 100% <ø> (ø) ⬆️
BaseLib/Histogram.h 0% <ø> (ø) ⬆️
MaterialLib/FractureModels/LogPenalty.h 65% <ø> (ø) ⬆️
...yPressure/BrooksCoreyCapillaryPressureSaturation.h 37.5% <ø> (ø) ⬆️
...ressure/BrooksCoreyCapillaryPressureSaturation.cpp 94.44% <ø> (ø) ⬆️
... and 306 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bff8ae...ecfddfb. Read the comment docs.

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Feb 23, 2019

@bilke I extracted the two commits for clang-tidy (partially) and clang-format configs; now in master.
I've another version of clang-tidy (also 7.0.1), which complains about the HeaderFilter option. It should be HeaderFilterRegex. Also the clang-tidy -dump-config suggests that HeaderFilterRegex is the correct one.

YAML:5:15: error: unknown key 'HeaderFilter'
HeaderFilter: '.*'
              ^~~~
Error parsing /home/naumov/w/ogs/s/.clang-tidy: Invalid argument

@endJunction endJunction force-pushed the bilke:clang-analyzer branch from 2cb2fd2 to e4b0156 Feb 23, 2019

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Feb 23, 2019

Tried with clang-tidy on my machine---it works too...

@endJunction endJunction force-pushed the ufz:master branch from 87041a1 to e1bf56f Feb 23, 2019

@endJunction endJunction force-pushed the bilke:clang-analyzer branch from e4b0156 to 746b761 Feb 23, 2019

@bilke bilke force-pushed the bilke:clang-analyzer branch 3 times, most recently from f54a8d6 to 480facc Feb 25, 2019

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Feb 26, 2019

Just an idea: The -fPIC mismatch error could be related to precompiled headers...

@bilke

This comment has been minimized.

Copy link
Member Author

bilke commented Feb 26, 2019

@endJunction No, I had to add -fpic to shapelib and cvode Conan packages.

Ready for review (everything is green except one failing unit test which sometimes fails).

@TomFischer
Copy link
Member

TomFischer left a comment

Looks good.

@endJunction endJunction force-pushed the bilke:clang-analyzer branch from b0ed37e to b451f12 Feb 26, 2019

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Feb 26, 2019

@ufz/members

I've added "few" of generic changes to the PR handling the clang-tidy checks regarding the

  • namespace comments
  • braces around if/else/for etc. blocks
  • and some of the else after return cases.

There will be conflicts with current PRs and other branches. If somebody has objections to the big changes, now is good time to talk about it.

@endJunction endJunction force-pushed the bilke:clang-analyzer branch 2 times, most recently from d404053 to 23a4d65 Feb 26, 2019

@bilke bilke force-pushed the bilke:clang-analyzer branch from 23a4d65 to 5edf2f8 Feb 27, 2019

@endJunction endJunction force-pushed the bilke:clang-analyzer branch from 5edf2f8 to 4e935b2 Feb 27, 2019

@endJunction endJunction force-pushed the bilke:clang-analyzer branch from 4e935b2 to ecfddfb Feb 27, 2019

@endJunction

This comment has been minimized.

Copy link
Member

endJunction commented Feb 27, 2019

@bilke Sometimes on a mac the 2D_Adaptive_dt_ThermalConvection_constviscosityStaggeredScheme ctest runs in just 62 seconds,
and sometimes times out after 900s. Some hidden work on a mac-slave?

passes in the last build, but time out in previous build.

@endJunction endJunction merged commit 073b0f9 into ufz:master Feb 27, 2019

3 of 5 checks passed

codecov/patch 23.07% of diff hit (target 32.69%)
Details
codecov/project 32.6% (-0.1%) compared to 1bff8ae
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details
ufz.ogs #20190227.13 succeeded
Details
@bilke

This comment has been minimized.

Copy link
Member Author

bilke commented Feb 27, 2019

@endJunction The passed build ran on vismac05 which is a current Mac model whereas the timed out benchmark ran on vismac02 which is from 2012 or so... No hidden work on the slaves.

@bilke bilke deleted the bilke:clang-analyzer branch Feb 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.