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

forward extra kwargs from py_venv to its internal targets #4

Merged
merged 1 commit into from
Nov 29, 2022
Merged

forward extra kwargs from py_venv to its internal targets #4

merged 1 commit into from
Nov 29, 2022

Conversation

getim
Copy link
Contributor

@getim getim commented Nov 25, 2022

The py_venv-macro does not accept any of Bazel's common build rule attributes because it limits itself to the 3 arguments it knows about. This PR makes it so that the macro accepts any extra arguments that are valid for both the internal _py_venv_deps and py_binary target and forwards them to those targets.

After this change, things like tags, visibility or testonly are supported on py_venv and have the expected behaviour.

)

py_binary(
name = name,
srcs = ["@rules_pyvenv//:build_env.py"],
deps = ["@rules_pyvenv//vendor/importlib_metadata"],
deps = ["@rules_pyvenv//vendor/importlib_metadata"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also fixed a line with some weird whitespace 🙂

@getim getim changed the title forwards extra kwargs from py_venv to its internal targets forward extra kwargs from py_venv to its internal targets Nov 25, 2022
Copy link
Contributor

@jvolkman jvolkman left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR!

@jvolkman jvolkman merged commit 115e8a4 into cedarai:main Nov 29, 2022
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