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

handle None versions #704

Merged
merged 1 commit into from Jun 24, 2017

Conversation

Projects
None yet
2 participants
@bgruening
Copy link
Member

bgruening commented Jun 24, 2017

@@ -159,7 +159,14 @@ def handle_pull_request(self, ctx, name, target_filename, packages_str, tools_st

def write_targets(self, ctx, target_filename, mulled_targets):
with open(target_filename, "w") as f:
contents = ",".join(["%s=%s" % (t.package_name, t.version) for t in mulled_targets])
target_strings = list()

This comment has been minimized.

@mvdbeek

mvdbeek Jun 24, 2017

Member

how about contents = ",".join(["%s=%s" % (t.package_name, t.version) if t.version else "%s" % t.package_name for t in mulled_targets]) ?

This comment has been minimized.

@bgruening

bgruening Jun 24, 2017

Author Member

I don't like these large list comprehensions, they are less readable and this section is not performance critical.

@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Jun 24, 2017

Is it a good idea to allow this though? Should we not better fix the tools?

@bgruening

This comment has been minimized.

Copy link
Member Author

bgruening commented Jun 24, 2017

I don't think such things are a good idea. But Galaxy allows this so this should work here as well. However, I have on my todo list to enhance planemo lint and if this all is merged I will fix the galaxyp tools.

@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Jun 24, 2017

Sounds good then!

@mvdbeek mvdbeek merged commit 9fc1296 into master Jun 24, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment