Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Bump fgbio versions and swap to nftest #5624
Bump fgbio versions and swap to nftest #5624
Changes from all commits
d6cc2b3
01bd4c3
3fc5a08
5ec0a95
3d38cc5
0bfe648
95ed61f
b1e1beb
7b18f7b
c3b5a1a
49922ba
abb2311
8d1dff3
559fb11
b0589f1
ceb8df1
4a99079
44458bc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you add this custom suffix here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs some kind of name renaming to prevent it from colliding with the input file. Some of the subtools already done this, some don't. Although this one is now setting a double prefix in the output file, so that needs fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add a check for that (a lot of modules do this already) => https://nf-co.re/docs/guidelines/components/modules#command-file-output-naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it needs the checks whatever.
I think it is better to have a default prefix that doesn't require you to add one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not one to argue with that, but I don't think diverting from the guidelines is a good idea, they're here for a reason. You can maybe bring this up on Slack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto, does the
main.nf
need to be updated?