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

Mention name of binary we can't find #4947

Merged
merged 2 commits into from Oct 6, 2017

Conversation

illicitonion
Copy link
Contributor

This gives better debugging information for what went wrong.

This gives better debugging information for what went wrong.
.format(os_id))
raise self.MissingMachineInfo(('Unable to find binary {name} version {version}. ' +
'Update --binaries-path-by-id to find binaries for {os_id!r}')
.format(name=name, version=version, os_id=os_id))
Copy link
Member

Choose a reason for hiding this comment

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

thanks to python's implicit string concatenation, you don't need the + or extra ()'s here:

>>> ('this is '
...  'an implicitly concatenated string '
...  'that can be formatted: {}'
...  .format('see'))
'this is an implicitly concatenated string that can be formatted: see'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really dislike implicit string concatenation :(

Done.

@@ -165,7 +165,7 @@ def uname_func():

with self.assertRaisesRegexp(BinaryUtil.MissingMachineInfo,
r'Pants has no binaries for vms'):
binary_util._select_binary_base_path("supportdir", "name", "version", uname_func=uname_func)
binary_util._select_binary_base_path("supportdir", "version", "name", uname_func=uname_func)
Copy link
Member

Choose a reason for hiding this comment

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

should all of the other callsites for _select_binary_base_path in this file invert name<->version too?

that seems to be the method signature:

def _select_binary_base_path(self, supportdir, version, name, uname_func=None):

so it's puzzling why other tests in this file pass them in the wrong order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Done.

Copy link
Member

@kwlzn kwlzn left a comment

Choose a reason for hiding this comment

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

lgtm

@kwlzn kwlzn merged commit e3fceaa into pantsbuild:master Oct 6, 2017
@illicitonion illicitonion deleted the dwagnerhall/findbinaryerror branch October 27, 2017 09:40
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.

None yet

2 participants