-
Notifications
You must be signed in to change notification settings - Fork 534
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
feat: add interpreter_version_info to py_runtime #1671
Conversation
245f06c
to
238de8e
Compare
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'll let other folks bikeshed if they want.
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.
My main comment is to use a dict/struct instead of list. This makes it easier to know which parts are check for the relevant pieces. Plus we can stash extra implementation-specific parts in there if necessary.
i.e. accept a attr.string_dict on the rule and and convert it to a struct() for the provider. Convert particular keys to their appropriate types (e.g. major/minor/patch to ints, similar to sys.version_info)
Please add tests. You can add them to //tests/py_runtime
238de8e
to
ef00f2b
Compare
ef00f2b
to
cee2188
Compare
cee2188
to
0090758
Compare
@rickeylev Can I have another pass on this please? |
Looks pretty good! Thank you for adding tests. Can you please add an entry to CHANGELOG.md? Then it's ready to merge. |
0090758
to
3a92745
Compare
…uild/rules_python into interpreter_version
Adds an
interpreter_version_info
attribute to thepy_runtime
and associated provider that maps to thesys.version_info
values. This allows the version of the interpreter to be known statically, which can be useful for rule sets that depend on the interpreter, and need to build environments / pathing that contain version info (virtualenvs for example).