Skip to content

Conversation

janwinkler1
Copy link
Contributor

Fixes target naming to follow symbolic macro conventions introduced in Bazel 8.0.

Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

  • Is there any way we can add a test for this?
  • Could you please add a CHANGELOG.md note to let people know that this fixes an issue?
@janwinkler1
Copy link
Contributor Author

@aignas thank you for your review!

sure, i am looking into it ATM.

in the dev-guide i read that integration tests are discuraged.

in c91459a i quickly tried to add a simpler build test, however that failed in CI, presumably because bazel 7 doesnt support symbolic macros.

so do you think in this case it would be okay to add a new integration test and ignore (if possible) bazel 7 tests there?

@aignas aignas mentioned this pull request Oct 11, 2025
7 tasks
@rickeylev
Copy link
Collaborator

re: testing this: Is adding target_compatible_with sufficient? Or does bazel 7 failure before it that is processed? If TCW isn't checked early enough, then how about wrapping it in a legacy macro, then checking the bazel version there, and skiping early? There's utility target in rules_testing for generating a no-op skip target for such cases; see skip_test in tests/py_runtime_pair/py_runtime_pair_tests.bzl for an example

re: losing the underscore prefix: We'll just have to live with it. We could add an infix or suffix to help denote its internal. e.g. foo_gen__ foo__gen foo_internal_gen foo_private_gen etc

@aignas aignas changed the title Make py_console_script_binary compatible with symbolic macros fix(rules): make py_console_script_binary compatible with symbolic macros Oct 11, 2025
@aignas aignas enabled auto-merge October 11, 2025 04:48
@aignas aignas added this pull request to the merge queue Oct 11, 2025
Merged via the queue into bazel-contrib:main with commit a2f14eb Oct 11, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants