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

refactor: broaden visibility and use list() instead of keys() #1704

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

rickeylev
Copy link
Collaborator

This upstreams a couple trivial Google patches to make rules_python more compatible with some Google-internal changes.

  • Use list(dict) instead of dict.keys(). This is because some bzl files are run through a Python-based testing framework, and dict.keys() can't be concatenated to a list in Python 3.
  • Expand visibility to all of rules_python instead of just the python subdirectory. This makes some patches that use internals easier to maintain, but also makes the visiblity more in line with the rest of the project (where //:__subpackages__ is used for convenience, as it avoids having to change visibility frequently, but still prevents public dependencies).

This upstreams a couple trivial Google patches to make rules_python
more compatible with some Google-internal changes.

* Use list(dict) instead of dict.keys(). This is because some bzl files
  are run through a Python-based testing framework, and dict.keys()
  can't be concatenated to a list in Python 3.
* Expand visibility to all of rules_python instead of just the
  python subdirectory. This makes some patches that use internals
  easier to maintain, but also makes the visiblity more in line
  with the rest of the project (where `//:__subpackages__` is
  used for convenience, as it avoids having to change visibility
  frequently, but still prevents public dependencies).
@rickeylev rickeylev requested a review from Solumin January 18, 2024 23:24
@rickeylev
Copy link
Collaborator Author

It looks like CI is unhappy because of some issue with 7.0.1 and the pystar implementation. Ugh. This is gonna be annoying to fix...

I'm just going to force-merge past this. The rest of CI OK.

@rickeylev rickeylev merged commit 711186f into bazelbuild:main Jan 18, 2024
3 of 4 checks passed
@rickeylev rickeylev deleted the upstream.patches branch January 18, 2024 23:47
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.

2 participants