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
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
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
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
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

@param patch_name: name of the patch file
"""
robot_path = build_option('robot_path')[0]
Copy link
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?

def copy_file(path, target_dir, soft_name, target_name):

target_path = det_location_for(path, target_dir, soft_name, target_name)
new = os.path.exists(target_path)
Copy link
Member

Choose a reason for hiding this comment

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

move this out of this function, this should b a generic function to copy files (also: add a docstring)



def copy_file(path, target_path):

Copy link
Member

Choose a reason for hiding this comment

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

docstring please!

raise EasyBuildError("Path doesn't exist or file to delete isn't found in target branch: %s", fn)

# copy edited/added files to right place
file_info = copy_easyconfigs(ec_paths, repo_path)
Copy link
Member

Choose a reason for hiding this comment

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

@Caylo: oops?

'validate': False,
})
self.mock_stdout(True)
ec = gh.scan_all_easyconfigs('toy-0.0_typo.patch')
Copy link
Member

Choose a reason for hiding this comment

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

this won't work :)

@boegel
Copy link
Member

boegel commented Aug 12, 2016

Going in, thanks @Caylo!

@boegel boegel merged commit f17c997 into easybuilders:develop Aug 12, 2016
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