-
Notifications
You must be signed in to change notification settings - Fork 660
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 ability to define templates library at module level #2332
Add ability to define templates library at module level #2332
Conversation
Signed-off-by: Manuele Simi <[email protected]>
… dir. Signed-off-by: Manuele Simi <[email protected]>
Signed-off-by: Manuele Simi <[email protected]>
Signed-off-by: Manuele Simi <[email protected]>
…m/manuelesimi/nextflow into manuelesimi-feature/loadTemplateFromModuleDir
Signed-off-by: Manuele Simi <[email protected]>
Signed-off-by: Manuele Simi <[email protected]>
Signed-off-by: Manuele Simi <[email protected]>
I added a I noticed how you tested the behavior of |
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.
Thinks looks great. Just made a comment on file template existence check.
modules/nextflow/src/main/groovy/nextflow/processor/TaskContext.groovy
Outdated
Show resolved
Hide resolved
Signed-off-by: Manuele Simi <[email protected]>
I'm just curious here, the approach used in this PR , could also be applied for |
@abhi18av The rational behind #1798 is indeed the same of this PR. They both aim to achieve self-contained module components. However, the solution found in this PR does not apply for the That said, solving #1798 would nicely pair with the solution we got here. |
Signed-off-by: Manuele Simi <[email protected]>
Ah, I see! Thanks for the detailed explanation @manuelesimi! |
Ok, think we can merge this. Nice contribution @manuelesimi, thanks! |
You're welcome! |
This PR replaces #2263 adding some minor refactoring and unit testing.