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

Fixes from Fortitude linter #190

Merged
merged 9 commits into from
Dec 12, 2024
Merged

Fixes from Fortitude linter #190

merged 9 commits into from
Dec 12, 2024

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented Nov 27, 2024

Applies fixes from Fortitude linter, see #189 for more info.

You can install fortitude with uv (or pip/pipx)

uv tool install fortitude-lint

And run it on the codebase

fortitude check src/

Most of the fixes are adding the intent attribute to functions/subroutine parameters, missing (or superfluous) implicit none, and missing kind parameter on number constants (0.5 -> 0.5D0)

In this PR I don't add Fortitude in CI (yet) since it is currently missing some configuration capability that we need. In particular, it's not possible to skip files. We need to skip FFTW3.F90 and force_cp2k.F90. The former is autogenerated file that we should not touch. The latter for some reason contains some weird errors that I was too lazy to understand (and this file is not really important now).

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 87.17949% with 5 lines in your changes missing coverage. Please review.

Project coverage is 92.84%. Comparing base (a781b39) to head (25917a4).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/force_spline.F90 0.00% 3 Missing ⚠️
src/force_bound.F90 50.00% 1 Missing ⚠️
src/shake.F90 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #190   +/-   ##
=======================================
  Coverage   92.84%   92.84%           
=======================================
  Files          47       47           
  Lines        6760     6761    +1     
  Branches      764      764           
=======================================
+ Hits         6276     6277    +1     
  Misses        472      472           
  Partials       12       12           
Flag Coverage Δ
unittests 20.37% <5.26%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/analysis.F90 98.85% <ø> (ø)
src/cmdline.F90 100.00% <ø> (ø)
src/ekin.F90 100.00% <ø> (ø)
src/en_restraint.F90 85.89% <100.00%> (ø)
src/error.F90 100.00% <ø> (ø)
src/fftw_interface.F90 84.84% <ø> (ø)
src/force_abin.F90 94.15% <100.00%> (ø)
src/force_h2o.F90 92.75% <100.00%> (ø)
src/force_tcpb.F90 29.72% <ø> (ø)
src/forces.F90 96.74% <100.00%> (ø)
... and 22 more

@danielhollas danielhollas changed the title Fixed from Fortitude linter Fixes from Fortitude linter Nov 30, 2024
@danielhollas danielhollas marked this pull request as ready for review December 11, 2024 21:06
Copy link
Contributor

@JanosJiri JanosJiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the changes look reasonable.

@danielhollas danielhollas merged commit 0750f7a into master Dec 12, 2024
19 checks passed
@danielhollas danielhollas deleted the fortitude branch December 12, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants