Skip to content

Remove re-exports of cosmic_text types #19516

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

Merged
merged 2 commits into from
Jun 6, 2025

Conversation

alice-i-cecile
Copy link
Member

Objective

As discussed in #19285, we do a poor job at keeping the namespace tidy and free of duplicates / user-conflicting names in places. cosmic_text re-exports were the worst offender.

Solution

Remove the re-exports completely. While the type aliases were quite thoughtful, they weren't used in any of our code / API.

@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 6, 2025
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Text Rendering and layout for characters X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 6, 2025
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 6, 2025
Copy link
Contributor

@SpecificProtagonist SpecificProtagonist left a comment

Choose a reason for hiding this comment

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

please rely on them directly from cosmic_text, being sure that the version number you are using matches the version used by your version of bevy_text.

Standard practice – which we are currently following, see the self in the import – would be to reëxport cosmic_text as bevy_text::cosmic_text to avoid this problem.

@alice-i-cecile
Copy link
Member Author

Standard practice – which we are currently following, see the self in the import – would be to reëxport cosmic_text as bevy_text::cosmic_text to avoid this problem.

Indeed, but I think the cure is worse than the disease here, and the others in the linked thread agree.

@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through and removed X-Uncontroversial This work is generally agreed upon labels Jun 6, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 6, 2025
Merged via the queue into bevyengine:main with commit e1230fd Jun 6, 2025
34 checks passed
VitalyAnkh pushed a commit to VitalyAnkh/bevy that referenced this pull request Jun 8, 2025
# Objective

As discussed in bevyengine#19285, we do a poor job at keeping the namespace tidy
and free of duplicates / user-conflicting names in places. `cosmic_text`
re-exports were the worst offender.

## Solution

Remove the re-exports completely. While the type aliases were quite
thoughtful, they weren't used in any of our code / API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Text Rendering and layout for characters C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants