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

Lint check for params.enable_conda #2213

Merged
merged 11 commits into from
Mar 29, 2023

Conversation

anoronh4
Copy link
Contributor

@anoronh4 anoronh4 commented Mar 27, 2023

Addresses #2187
usage of params.enable_conda has been deprecated. Currently it is a fail condition from the pipeline linter which only checks the config file but the module linter does not.

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

Tests were updated to catch new failures.

@github-actions
Copy link
Contributor

This PR is against the master branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @anoronh4,

It looks like this pull-request is has been made against the anoronh4/tools master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the anoronh4/tools dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@anoronh4 anoronh4 changed the base branch from master to dev March 27, 2023 19:03
@anoronh4 anoronh4 marked this pull request as ready for review March 27, 2023 20:46
@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #2213 (ff84a09) into dev (14951aa) will increase coverage by 0.34%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##              dev    #2213      +/-   ##
==========================================
+ Coverage   72.97%   73.31%   +0.34%     
==========================================
  Files          78       78              
  Lines        8380     8384       +4     
==========================================
+ Hits         6115     6147      +32     
+ Misses       2265     2237      -28     
Impacted Files Coverage Δ
nf_core/modules/lint/main_nf.py 66.20% <88.88%> (+0.47%) ⬆️
nf_core/__main__.py 58.97% <100.00%> (ø)

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@anoronh4
Copy link
Contributor Author

looks like one of the tests is failing for API rate limit reasons:

DEBUG    nf_core.utils:utils.py:485 {
    "message": "API rate limit exceeded for 20.172.6.193. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",

So we'll have to just retry later!

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.

Looks good :) Just some small comments. Let's see if the test passes. If you want, you can also add a check on the pipeline linting.

nf_core/modules/lint/main_nf.py Outdated Show resolved Hide resolved
nf_core/modules/lint/main_nf.py Outdated Show resolved Hide resolved
nf_core/modules/lint/main_nf.py Show resolved Hide resolved
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.

Thank you :) Left one last comment to address, but after that looks good to me

nf_core/modules/lint/main_nf.py Show resolved Hide resolved
@mirpedrol
Copy link
Member

@nf-core-bot fix linting

@anoronh4 anoronh4 requested a review from mirpedrol March 29, 2023 22:43
@anoronh4 anoronh4 merged commit bb533fc into nf-core:dev Mar 29, 2023
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