Fix subworkflows update command to work for non-nf-core repos #2103
Conversation
|
@fmalmeida Any chance you can pull this code and see if it fixes your issue? ( |
Codecov Report
@@ Coverage Diff @@
## dev #2103 +/- ##
=======================================
Coverage 67.97% 67.97%
=======================================
Files 43 43
Lines 5633 5633
=======================================
Hits 3829 3829
Misses 1804 1804
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Hi, |
| component if directory == self.modules_repo.repo_path else f"{directory}/{component}" | ||
| for directory, component in components |
There was a problem hiding this comment.
| component if directory == self.modules_repo.repo_path else f"{directory}/{component}" | |
| for directory, component in components | |
| component for _, component in components |
I'm not sure how to test that right now, but I think we can directly remove the whole if/else. Actually, this was a comment on the original PR, but we decided to change it later.
This will have some problems if there are two modules with the same name, but TBH I think we will have many other problems if that happens, so we should handle it differently.
If a user provides the name of a module without specifying the directory, the first code will fail. Not sure if that was the case for this particular issue, but I would say everyone will expect it to work with only the tool name.
There was a problem hiding this comment.
I don't think this makes a difference unless we change how we initially get components right? (Which right now is passed a repo_url).
This is part of a wider change I would like to make (probably in 2.8) where other than install we shouldn't need to rely on the remote repo being passed to the command with --git-repo (we already made it that way for lint I think?)
I have some experimental work that returns displays the module/subwf choice with it's "org" too, but for now that still didn't fix underlying issues if two modules/subwf had the same name so I haven't progressed it
There was a problem hiding this comment.
Makes sense, let's leave these big changes for 2.8 👍
Fixes #2101
PR checklist
CHANGELOG.mdis updateddocsis updated