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

Allow customization of Python rules #458

Open
ahans opened this issue Mar 20, 2025 · 3 comments · May be fixed by #460
Open

Allow customization of Python rules #458

ahans opened this issue Mar 20, 2025 · 3 comments · May be fixed by #460

Comments

@ahans
Copy link
Contributor

ahans commented Mar 20, 2025

I have a use case where I'd like to use the py_binary from rules_py as the backend of ros2_py_binary instead of the default one from rules_python. My current solution is a local patch. Would there be any issues with allowing customization from the user? We could probably simply expose the target parameter and default it to None. If it's set, we'll use it. Otherwise, we use the variant from rules_python just like today. What do you think? Am I missing anything? If not, I would put up a PR with this change.

@mvukov
Copy link
Owner

mvukov commented Mar 23, 2025

Fine by me, please create a PR.

@mvukov
Copy link
Owner

mvukov commented Mar 23, 2025

Just curious, why do you patch this repo at the moment? You could juse pass target argument to https://github.com/mvukov/rules_ros2/blob/main/ros2/py_defs.bzl#L8.

@ahans
Copy link
Contributor Author

ahans commented Mar 24, 2025

Just curious, why do you patch this repo at the moment? You could juse pass target argument to https://github.com/mvukov/rules_ros2/blob/main/ros2/py_defs.bzl#L8.

I actually tried that, but Bazel told me I was not allowed to use that macro. Looks like macros with leading underscore are automatically made private.

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 a pull request may close this issue.

2 participants