-
Notifications
You must be signed in to change notification settings - Fork 202
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
add flexibility support for specifying OS dependencies #846
Conversation
@stdweird: review? |
Hi Ken, I have 2nd thoughts about this PR, it makes the assumption that there is 1-1 mapping among osdeps of the various distros. Boost is a notable example which can come from 1 to N packages, and N can vary a lot. Of course, we can always claim this is a problem to be handled in the future. |
if not self._os_dependency_check(dep): | ||
not_found.append(dep) | ||
if isinstance(dep, basestring): | ||
if not self._os_dependency_check(dep): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't like this. if it is a basestring, make it a tuple, and use the tuple code. basestring shoul dbe considered shorthand for single element tuple. so always process a tuple to avoid code duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, agreed, will change.
@boegel minor remark |
@fgeorgatos: Hmm, good point... We can extend this notion to allow for: osdependencies = [('boost', ['boost-dev', 'libboost'])] but I'm not sure if we want to take that road... @stdweird: Did you catch @fgeorgatos' remark? |
@fgeorgatos @boegel yes, i understand it, but this PR can be useful even in current form (eg |
@stdweird, @fgeorgatos: remark fixed, and like @stdweird mentioned, more involved cases can be handled as deemed required; this should provide enough flexibility for now |
add flexibility support for specifying OS dependencies
No description provided.