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.

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

Remove md5sum for versions.yml file #1511

Merged
merged 10 commits into from
Apr 15, 2022
Merged

Remove md5sum for versions.yml file #1511

merged 10 commits into from
Apr 15, 2022

Conversation

mirpedrol
Copy link
Member

@mirpedrol mirpedrol commented Apr 12, 2022 •
edited by ewels
Loading

Remove md5sum for versions.yml file when using create_test_yml

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

@github-actions

This comment was marked as resolved.

Sign in to view
@codecov

This comment was marked as resolved.

Sign in to view
@mirpedrol mirpedrol changed the base branch from master to dev April 12, 2022 10:44
@FriederikeHanssen
Copy link
Contributor

I am not sure why the branch protection test is failing after changing to dev branch

@ewels
Copy link
Member

ewels commented Apr 13, 2022

I am not sure why the branch protection test is failing after changing to dev branch

It always needs a new commit (push or rebase) to run properly again and pass

ewels
ewels reviewed Apr 13, 2022
View reviewed changes
Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Very minor style suggestion, otherwise looks great! (I haven't tested locally, but looks safe enough).

Also added a comment about an opportunity to do a bit of refactoring 👀 😅

CHANGELOG.md Outdated Show resolved Hide resolved
nf_core/modules/test_yml_builder.py Outdated Show resolved Hide resolved
nf_core/modules/test_yml_builder.py Outdated Show resolved Hide resolved
@ewels ewels added command line tools Anything to do with the cli interfaces modules Related to tools for working with DSL2 modules labels Apr 13, 2022
mirpedrol and others added 2 commits April 13, 2022 12:51
@mirpedrol @ewels
Update CHANGELOG.md
cd78ef9 
Co-authored-by: Phil Ewels <phil.ewels@scilifelab.se>
@mirpedrol @ewels
Use continue in if statement
708c807 
Co-authored-by: Phil Ewels <phil.ewels@scilifelab.se>
@ewels
Copy link
Member

ewels commented Apr 13, 2022

ooh, let's try this out! @nf-core-bot fix linting

@ewels
Copy link
Member

ewels commented Apr 13, 2022

hah, of course. Needs to be on the default branch so we can't actually test that until we do a release 🤦🏻

Ok, you need to fix the Python black linting yourself this time then sorry 😅 (but looks like you have a syntax error, so likely the error is due to that)

@mirpedrol mirpedrol requested a review from ewels April 13, 2022 13:44
@mirpedrol @ewels
restore deleted lines
7772e5a 
Co-authored-by: Phil Ewels <phil.ewels@scilifelab.se>
ewels
ewels approved these changes Apr 15, 2022
View reviewed changes
Copy link
Member

@ewels ewels 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!

@ewels ewels merged commit faf4716 into nf-core:dev Apr 15, 2022
@mirpedrol mirpedrol deleted the test_yml branch May 17, 2022 12:57
@muffato muffato mentioned this pull request Jul 3, 2023
Merged
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ewels ewels ewels approved these changes

Assignees
No one assigned
Labels
command line tools Anything to do with the cli interfaces modules Related to tools for working with DSL2 modules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mirpedrol @FriederikeHanssen @ewels