-
Notifications
You must be signed in to change notification settings - Fork 192
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
🩹 added patch command for subworkflows #2861
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
for testing you can do similar things to https://github.com/nf-core/tools/blob/master/tests/modules/patch.py |
Hello! @ctuni are you planning to work on this PR, otherwise I would like to take over and finish it up :) |
Hello! Thank you for reviving this, please go ahead and let me know if I can help with anything! |
@ctuni this might help your future development work 😉 pip install pre-commit
pre-commit install It'll run |
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.
Changelog entry is missing
if subworkflow.is_patched: | ||
# If the subworkflow is patched, we need to apply | ||
# the patch in reverse before comparing with the remote | ||
tempdir_parent = Path(tempfile.mkdtemp()) | ||
tempdir = tempdir_parent / "tmp_subworkflow_dir" | ||
shutil.copytree(subworkflow.component_dir, tempdir) | ||
try: | ||
new_lines = ComponentsDiffer.try_apply_patch( | ||
subworkflow.component_type, | ||
subworkflow.component_name, | ||
subworkflow.org, | ||
subworkflow.patch_path, | ||
tempdir, | ||
reverse=True, | ||
) | ||
for file, lines in new_lines.items(): | ||
with open(tempdir / file, "w") as fh: | ||
fh.writelines(lines) | ||
except LookupError: | ||
# This error is already reported by subworkflow_patch, so just return |
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.
wtihout checking, this feels like we would also use the same logic inn modules_changes
. can we make a components_changes
superfunction?
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 think this is part of a bigger restructure where we have a components lint instead of splitting modules and subworkflows
Related to issue: #1891
Is there a straightforward way to test this?