Skip to content

also include patch files in --new-pr and --update-pr#1852

Merged
boegel merged 14 commits intoeasybuilders:developfrom
Caylo:new_pr_patch
Aug 12, 2016
Merged

also include patch files in --new-pr and --update-pr#1852
boegel merged 14 commits intoeasybuilders:developfrom
Caylo:new_pr_patch

Conversation

@Caylo
Copy link
Copy Markdown
Contributor

@Caylo Caylo commented Jul 25, 2016

building on #1751
cfr #1674

@boegel boegel added this to the v2.9.0 milestone Aug 3, 2016
Comment thread easybuild/tools/github.py Outdated
if soft_name:
patch_specs.append((patch_path, soft_name))
else:
raise EasyBuildError("Failed to determine software name to which patch file %s relates", patch_path)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Caylo please hoist this blob of code that determines patch_specs into a separate function, it'll help make things clearer since it's rather big (and will get bigger if the FIXME is handled)

@param target_dir: (parent) target directory, should contain easybuild/easyconfigs subdirectory
@param soft_name: software name (to determine location to copy to)
@param target_file: target file name
@return: full path to the right location
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/@param/:param/

@ocaisa ocaisa mentioned this pull request Aug 11, 2016
11 tasks
Comment thread easybuild/tools/github.py Outdated

@param patch_name: name of the patch file
"""
robot_path = build_option('robot_path')[0]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the [0]? you're only considering the first entry in the robot search path? why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants