-
Notifications
You must be signed in to change notification settings - Fork 16
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 scripts to extract workflows from WorkflowHub and UseGalaxy servers #158
Conversation
Fixed the linting, don't have time to look at the rest at the moment, sorry! |
Thanks already for fixing the linting!! |
4dcc114
to
d00ddcb
Compare
- Add matrix strategy for test tutorial and test workflows - Replace pull_request_target by pull_request
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.
Could you add the output of a test run ? That would make the review simpler. Will try to check tomorrow.
Shorten tool id | ||
""" | ||
if "toolshed" in tool: | ||
return tool.split("/")[-2] |
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.
are you sure it's not [-1]
?
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.
If we want to have the suite id it is [-2]
and it is what we used there: https://github.com/galaxyproject/galaxy_codex/pull/158/files#diff-fded0df90a63fa85d73a79a61797665ed61e1b1a921558f0d3b301d877a54dc4L38
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.
True, I just remembered we do it different for the tool stats (but there we compare wrapper ids - so all good !)
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.
Looks good and I like the OO approach, could you provide the test output, still ?
Shorten tool id | ||
""" | ||
if "toolshed" in tool: | ||
return tool.split("/")[-2] |
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.
True, I just remembered we do it different for the tool stats (but there we compare wrapper ids - so all good !)
workflow CI fails, with
locally it worked for me, maybe we need to retry this a few times ... ? |
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.
Really nice ! Thanks @bebatut !
No description provided.