-
Notifications
You must be signed in to change notification settings - Fork 191
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
Add --version to NfCoreTemplate #1951
Conversation
Hi @SamStudio8, that's a good solution! |
Hi @mirpedrol! I think if this were to get added to the template it should be all-or-nothing to ensure it provides predictable behaviour. That is, we should either add it to the template and force all pipelines to make use of --version; or not add it at all. |
For what it's worth, pipelines cannot use their own
(I've just had to patch out this very thing to make this PR work) |
Codecov Report
@@ Coverage Diff @@
## dev #1951 +/- ##
=======================================
Coverage 63.91% 63.91%
=======================================
Files 43 43
Lines 5509 5509
=======================================
Hits 3521 3521
Misses 1988 1988 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Hi, sorry for the long pause. I think it might lead to some confusion that |
No worries @mashehu!
Either way, I'd push for |
I'd go for |
Besides the naming discussion, is this PR ready? Or still WIP? I've tested it and works nicely 🙂 |
Hi @mirpedrol, I was thinking of moving this version function to the NfcoreTemplate.groovy so it could be used in the other functions that refer to the workflow version (email, logo)? WorkflowMain already calls functions in NfcoreTemplate so it would not be unusual. What do you think? |
de5ffeb
to
939e2ba
Compare
@mirpedrol I've just bumped this to follow up on my previous suggestion, I think it makes sense for anything reporting the workflow version to report the same thing. Let me know what you think, I can always revert the commit. |
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.
LGTM!
And I also think it makes sense moving the function to NfcoreTemplate. I like how it prints the version together with the logo :)
939e2ba
to
f4aee49
Compare
f4aee49
to
960e5cf
Compare
Thanks @mirpedrol! I've rebased and squashed so I think this is ready for CI 🚀 |
I don't seem to have perms to poke the CI pipe but we seem to have been caught up in the API rate limit! |
all green now! 🚀 |
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.
me likey!
As briefly discussed in the nf-core Slack, I was recently asked by a user "how do I know what version I have of your workflow?" which seemed like a simple question!
I was surprised to find that
nextflow info <workflow>
does not provide this (but I can imagine why).nextflow info
does list and show the current revision, which is fine if you've checked out a specific tag but not useful for users who have just pulled withnextflow run <workflow>
without-r
.The
workflow.manifest.version
is used by nf-core's template to populate an email message, and the logo function; but I wonder whether a more familiar--version
-like invocation of a pipeline to print out version information might also be useful?The WorkflowMain.version function definition in this PR will cook up a version identifier using both the manifest, and the workflow commitId (if available), it might be that this function should move to NfcoreTemplate instead to allow it to be used in the message and logo functions too.
Notably this would prevent any pipeline from using
version
as a parameter, so might be one to think about...! Thoughts welcome!Examples below:
PR checklist
CHANGELOG.md
is updateddocs
is updated