User modern rules_python and expect users to provide the mypy library themselves #82
Conversation
|
@alexeagle pinging you directly since you created several commits in the recentish history of this project. Any opinion on this contribution? |
BUILD
Outdated
| # No usable default. Force users to inject mypy. | ||
| build_setting_default = "//tools:empty_library", |
There was a problem hiding this comment.
could we just use a default mypy version? then it will still "just work" for anyone who pull this in and just tries it, while still allowing for easy overriding
There was a problem hiding this comment.
I would recommend not doing so. The reason is, that using a default mypy requires fetching it via rules_python and pip_parse, which makes using this project via a WORKSPACE file quite clunky. Essentially this would require calling the multiple steps to load mypy with some names hard coded in this project.
On the other hand, most likely only Python users will use this project at all, so adding mypy to their deps and setting the label_flag are literally 2 lines of work.
.bazelrc
Outdated
| @@ -0,0 +1 @@ | |||
| build --//:mypy=//third_party:mypy | |||
There was a problem hiding this comment.
if we use a valid default for the mypy version then we don't have to set this here
repositories/repositories.bzl
Outdated
|
|
||
| # buildifier: disable=function-docstring | ||
| def repositories(): | ||
| excludes = native.existing_rules().keys() | ||
| versions = struct( | ||
| rules_python = "0.18.1", |
There was a problem hiding this comment.
Can we jump straight to 0.27.1? That is the last version that supports Bazel 5. Bazel 4 is already end-of-life, so I don't think we need to support it anymore.
There was a problem hiding this comment.
If you want that done in one big step with the other changes, i can do so
There was a problem hiding this comment.
Either way! We can always upgrade further in another PR
|
Did you mean to close this? Looks like GitHub auto-closed it. |
Some time ago I created a fork of this project to be able to utilize it in a project of mine which uses a fairly recent version of rules_python. However, the current version of this project is not compatible modern rules_python versions. Since the fork worked for me well for some time now I want ti contribute this to the main repo so everybody can profit from it.
The main changes are:
pip_installwill be removed. Onlypip_parsewill work in the future.mypylibrary themselvesmypylibrary via alabel_flagis an elegant interface and fits into the pattern how the project already allows setting a mypy config.I believe these changes are also an important building block to enable
bzlmodfor this project, which would make using it even more convenient. If this PR is accepted I would also try to help with adopting bzlmod.I am looking forward to your review.