Skip to content

Conversation

mai93
Copy link
Contributor

@mai93 mai93 commented Sep 16, 2025

Remove _native_rules_allowlist attribute as it is no-op

This is needed to allow removing the deprecated flag --native_rules_allowlist from bazel.

Work towards: #3252

@mai93 mai93 marked this pull request as ready for review September 16, 2025 19:04
@rickeylev
Copy link
Collaborator

@gemini-code-assist /review

@gemini-code-assist Check for code that uses the _native_rules_allowlist attribute elsewhere in the repository.

Copy link
Contributor

Hello @rickeylev, I'm performing the requested code review now.

Regarding your request to "Check for code that uses the _native_rules_allowlist attribute elsewhere in the repository," I am unable to perform repository-wide searches. My review capabilities are limited to the changes presented within this pull request's diffs. I recommend using your SCM's search functionality or local tools to perform such a search.


Code Review

This pull request aims to remove the _native_rules_allowlist attribute, as it is considered a no-op and is related to a deprecated flag. The changes are well-aligned with the stated objective.

Summary of Changes:

  • python/private/attributes.bzl: The _native_rules_allowlist attribute, its associated loading of py_internal, and the _create_native_rules_allowlist_attrs function have been completely removed. This is consistent with the attribute being a no-op.
  • python/private/py_runtime_rule.bzl: The import and usage of NATIVE_RULES_ALLOWLIST_ATTRS have been removed, which is a necessary cleanup after its removal from attributes.bzl.

Review Criteria Assessment:

  • Correctness: The removal of a no-op attribute and its related code seems correct, especially given the context of deprecating the associated flag. The changes are consistent across the affected files.
  • Maintainability: Removing dead code and unnecessary imports improves the overall maintainability and reduces cognitive load for future developers.

No issues were found within the scope of the provided diffs that would prevent this change from being merged.

Great work on this cleanup!

@rickeylev
Copy link
Collaborator

Huh. Is this attribute entirely unused? I thought there was a helper function that checked it, but a quick search didn't turn it up.

@rickeylev rickeylev added this pull request to the merge queue Sep 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 16, 2025
@aignas aignas added this pull request to the merge queue Sep 17, 2025
Merged via the queue into bazel-contrib:main with commit 15b5da7 Sep 17, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants