Skip to content

Misc. changes to nf-core modules install #1190

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 12, 2021
Merged

Misc. changes to nf-core modules install #1190

merged 3 commits into from
Jul 12, 2021

Conversation

@ErikDanielsson
Copy link
Contributor

@ErikDanielsson ErikDanielsson commented Jul 12, 2021

Misc. changes to install:

  • Added check that the remote of a module exists.
  • Made --latest default for modules install --all. It is still possble to override with --sha
  • Remove unnecessary critical error from nf-core modules install.
  • Added try-except blocks in a few more places.

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

drpatelh
drpatelh reviewed Jul 12, 2021
View reviewed changes
README.md
* `--force`: Overwrite a previously installed version of the module.
* `--sha <commit_sha>`: Install the module at a specific commit from the `nf-core/modules` repository.
* `--all`: Use this flag to change versions on all installed modules. Has the same effect as running `nf-core modules install --force` on all installed modules.
* `--all`: Use this flag to change versions on all installed modules. Has the same effect as running `nf-core modules install --force --latest` on all installed modules. To change all modules to a specific version you can run `nf-core modules install --all --sha <commit sha>`.
Copy link
Member

@drpatelh drpatelh Jul 12, 2021

Choose a reason for hiding this comment

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

Suggested change
* `--all`: Use this flag to change versions on all installed modules. Has the same effect as running `nf-core modules install --force --latest` on all installed modules. To change all modules to a specific version you can run `nf-core modules install --all --sha <commit sha>`.
* `--all`: Use this flag to change versions on all installed modules. Has the same effect as running `nf-core modules install --force` on all installed modules. To change all modules to a specific version you can run `nf-core modules install --all --sha <commit sha>`.

Copy link
Member

@drpatelh drpatelh Jul 12, 2021

Choose a reason for hiding this comment

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

Sorry, I thought we were removing --latest altogether as an option?

Copy link
Contributor Author

@ErikDanielsson ErikDanielsson Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No, the --latest flag should be included. Because we don't want to remove the cli prompt entirely right?

Copy link
Member

@drpatelh drpatelh Jul 12, 2021

Choose a reason for hiding this comment

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

Ah, I see. So its one of those options we need to keep for the CLI prompt. Finally got there! 😅 Ok.

@codecov
Copy link

codecov bot commented Jul 12, 2021 •
edited
Loading

Codecov Report

Merging #1190 (e3cbb9e) into dev (1bf0832) will decrease coverage by 0.21%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1190      +/-   ##
==========================================
- Coverage   70.31%   70.10%   -0.22%     
==========================================
  Files          49       49              
  Lines        5289     5312      +23     
==========================================
+ Hits         3719     3724       +5     
- Misses       1570     1588      +18
Impacted Files Coverage Δ
nf_core/__main__.py 55.86% <ø> (+0.14%) ⬆️
nf_core/modules/install.py 38.82% <20.00%> (-3.67%) ⬇️
nf_core/modules/module_utils.py 46.91% <50.00%> (+0.46%) ⬆️

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 1bf0832...e3cbb9e. Read the comment docs.

drpatelh
drpatelh approved these changes Jul 12, 2021
View reviewed changes
Copy link
Member

@drpatelh drpatelh left a comment

Choose a reason for hiding this comment

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

LGTM!

@drpatelh drpatelh merged commit 14cc1fe into nf-core:dev Jul 12, 2021
@ErikDanielsson
Copy link
Contributor Author

ErikDanielsson commented Jul 12, 2021

Thanks for the review @drpatelh!

@ErikDanielsson ErikDanielsson deleted the make-errors-nicer branch July 26, 2022 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@drpatelh drpatelh drpatelh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ErikDanielsson @drpatelh