lsmo-epfl / aiida-lsmo

AiiDA workflows for the LSMO laboratory at EPFL
Other
9 stars 13 forks source link

Fix cp2kbindingenergytest #86

Closed mpougin closed 3 years ago

mpougin commented 3 years ago

85

fixes as discussed in issue

codecov-commenter commented 3 years ago

Codecov Report

Merging #86 (fb84a9d) into develop (526b120) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop      #86   +/-   ##
========================================
  Coverage    76.40%   76.40%           
========================================
  Files           33       33           
  Lines         2530     2530           
========================================
  Hits          1933     1933           
  Misses         597      597           

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 526b120...fb84a9d. Read the comment docs.

ltalirz commented 3 years ago

thanks, just a minor pre-commit nitpick https://github.com/lsmo-epfl/aiida-lsmo/pull/86/checks?check_run_id=2611058143#step:5:37

you'll see those locally if you do

pip install -e .[pre-commit]
pre-commit install
mpougin commented 3 years ago

thanks, just a minor pre-commit nitpick https://github.com/lsmo-epfl/aiida-lsmo/pull/86/checks?check_run_id=2611058143#step:5:37

you'll see those locally if you do

pip install -e .[pre-commit]
pre-commit install

thanks for your remarks @ltalirz. stupid question, not sure if I understand properly: the pre-commit test failed because I didn't have pre-commit installed, which looks for minor issues before submission. Is this correct? I installed it now, do I have to redo everything?

ltalirz commented 3 years ago

thanks for your remarks @ltalirz. stupid question, not sure if I understand properly: the pre-commit test failed because I didn't have pre-commit installed, which looks for minor issues before submission. Is this correct?

Correct.

I installed it now, do I have to redo everything?

By default, pre-commit only scans the files that have uncommitted changes - therefore if you don't touch the file with the reported issue it won't complain (one can always run it on the whole codebase with pre-commit run --all-files but that can take a bit of time).

I.e. simply make the fix that was suggested and commit - if the pre-commit checks pass locally, so should they on github

ltalirz commented 3 years ago

thanks @mpougin !