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

Support patch file in nf-core modules lint #1716

Merged
merged 19 commits into from
Aug 1, 2022

Conversation

ErikDanielsson
Copy link
Contributor

@ErikDanielsson ErikDanielsson commented Aug 1, 2022

Continuation from #1708 and #1710. This PR adds support for patch files when linting modules. I have added a new module lint test module_patch that checks if a module's patch file is valid and reversible (i.e. we can construct the original files from it). In the other lint tests, the patch is reversed before performing any checking. However, I'm not sure if this is the optimal approach: would it be better in for example the main_nf test to lint the modified file and only report warnings if the lint fails?

I've waited with adding tests for patches in nf-core lint, since I want to do it in a similar fashion as in #1708 and #1710. This is not possible currently due to #1715.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Merging #1716 (913c7cd) into dev (fb57fb9) will decrease coverage by 0.74%.
The diff coverage is 46.96%.

❗ Current head 913c7cd differs from pull request most recent head f63a171. Consider uploading reports for the commit f63a171 to get more accurate results

@@            Coverage Diff             @@
##              dev    #1716      +/-   ##
==========================================
- Coverage   68.99%   68.24%   -0.75%     
==========================================
  Files          58       59       +1     
  Lines        6960     7080     +120     
==========================================
+ Hits         4802     4832      +30     
- Misses       2158     2248      +90     
Impacted Files Coverage Δ
nf_core/lint/__init__.py 73.85% <0.00%> (ø)
nf_core/modules/lint/module_patch.py 12.98% <12.98%> (ø)
nf_core/modules/lint/module_changes.py 52.38% <40.00%> (-30.96%) ⬇️
nf_core/modules/lint/meta_yml.py 63.82% <64.70%> (+0.67%) ⬆️
nf_core/modules/lint/main_nf.py 64.61% <69.23%> (+0.44%) ⬆️
nf_core/modules/modules_differ.py 86.00% <78.12%> (-0.67%) ⬇️
nf_core/modules/nfcore_module.py 90.90% <81.81%> (-4.93%) ⬇️
nf_core/modules/lint/__init__.py 84.05% <100.00%> (-0.74%) ⬇️
nf_core/modules/patch.py 81.81% <100.00%> (+0.33%) ⬆️
nf_core/modules/update.py 78.45% <100.00%> (-0.60%) ⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

LGTM thanks! :)

nf_core/modules/lint/meta_yml.py Outdated Show resolved Hide resolved
nf_core/modules/lint/module_patch.py Outdated Show resolved Hide resolved
nf_core/modules/modules_differ.py Outdated Show resolved Hide resolved
nf_core/modules/patch.py Outdated Show resolved Hide resolved
@ErikDanielsson ErikDanielsson merged commit 92211f3 into nf-core:dev Aug 1, 2022
@ErikDanielsson
Copy link
Contributor Author

Thanks for the review!

@ErikDanielsson ErikDanielsson deleted the lint-patch branch August 1, 2022 16:07
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