Skip to content
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

Correctly check set_environment declarations when updating repository. #2116

Merged
merged 1 commit into from Apr 22, 2016

Conversation

Projects
None yet
4 participants
@davebx
Copy link
Contributor

commented Apr 8, 2016

This pull request addresses #2111.

# The tool dependency is current, so keep it.
return False
if td_key == 'set_environment':
for td in td_val:

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Apr 9, 2016

Member

@davebx
I don't think we should check the set_environment key for removable tool_dependencies, since this checks if a tool_dependency can be removed, where set_environment is already part of that tool_dependency.

if td_key == 'set_environment':
    continue

should do it.

This comment has been minimized.

Copy link
@davebx

davebx Apr 22, 2016

Author Contributor

@mvdbeek I recently tested that, and determined that the set_environment tool dependency would disappear if its value was updated, but the updated version would not be added.

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Apr 22, 2016

Member

Hmm, I'm having trouble understanding this, I would appreciate an example to help me here,
but I'm also having a hard time to come up with a scenario where a false negative would be a problem, so 👍

@martenson martenson added this to the 16.07 milestone Apr 12, 2016

@mvdbeek mvdbeek merged commit f23c3ae into galaxyproject:dev Apr 22, 2016

4 checks passed

api test Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished.
Details
toolshed test Build finished.
Details
@mvdbeek

This comment has been minimized.

Copy link
Member

commented Apr 22, 2016

Thanks @davebx !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.