Allow repo rules to define previously reserved attributes#28453
Allow repo rules to define previously reserved attributes#28453michaelm-openai wants to merge 3 commits intobazelbuild:masterfrom
Conversation
github-actions
Bot
added
team-ExternalDeps
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request allows repository rules to define attributes that were previously reserved, specifically tags, deprecation, and visibility. The implementation correctly reduces the set of reserved attribute names to just name. The change is accompanied by new integration tests that verify a repository rule can now define and use a visibility attribute. The changes are clean, correct, and improve the flexibility of repository rule definitions. I have no specific comments as the implementation and tests look good.
meteorcloudy
added
awaiting-PR-merge
| Maps.filterKeys(kwargs, k -> !LEGACY_BUILTIN_ATTRIBUTES.contains(k)), | ||
| Maps.filterKeys( | ||
| kwargs, | ||
| k -> !RESERVED_ATTRIBUTES.contains(k)), |
There was a problem hiding this comment.
Nit: Since name is already handled specially above, we could just get rid of RESERVED_ATTRIBUTES and just check for name (extracted into a String constant) directly.
There was a problem hiding this comment.
/gemini Remove RESERVED_ATTRIBUTES and just check whether k is not equal to "name".
There was a problem hiding this comment.
Hoped that would work, oh well. Manually made the update.
github-actions
Bot
removed
the
awaiting-PR-merge
3 participants
Fixes #28450