refactor: avoid star-imports in analytics package (#4927) - #4933
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the analytics package to eliminate star-import re-export leakage from report_elements.py, and updates the static-site fetcher to import ADDITIONAL_DATA_BEHAVIOR explicitly so static-site generation does not depend on accidental re-exports.
Changes:
- Replace
report_elements.pystar imports with explicit imports from_report_utilsand anentitiesmodule alias. - Update
static_site/fetch.pyto import and useADDITIONAL_DATA_BEHAVIORdirectly fromentities. - Bump analytics package version to
5.0.4.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| analytics/analytics_package/setup.py | Bumps the analytics package version for the refactor. |
| analytics/analytics_package/analytics/static_site/fetch.py | Stops relying on report_elements re-exports by importing ADDITIONAL_DATA_BEHAVIOR directly. |
| analytics/analytics_package/analytics/report_elements.py | Removes star imports and prefixes entity constants to reduce unintended public exports. |
Suppressed comments (1)
analytics/analytics_package/analytics/report_elements.py:9
from . import entities as eintroduces a new public nameeonreport_elements(including onimport *), which is the same kind of module-alias export leak called out in #4927. Consider renaming this to a private alias (e.g.,_entities) or adding__all__so only the intended symbols are exported.
from . import entities as e
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
NoopDog
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4927
For the
entitiesimport, I decided that importing the whole module under the aliasewould be nicer that listing out every imported constant, which means that most of the changes toreport_elements.pyare prefixing the constants withe.