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.

Sign up for GitHub

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

Jump to bottom

Make get_short_name return Cow instead of String (Adopted) #12275

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make get_short_name return Cow instead of String (Adopted) #12275

wants to merge 1 commit into from
+94 −81

Conversation

tygyh
Copy link
Contributor

@tygyh tygyh commented Mar 3, 2024 •
edited by james7132
Loading

@tygyh tygyh changed the title Make get_short_name return Cow instead of String Make get_short_name return Cow instead of String (Adopted) Mar 3, 2024
@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-Utils Utility functions and types labels Mar 3, 2024
@james7132
Copy link
Member

james7132 commented Mar 3, 2024 •
edited
Loading

Can you fix the broken test?

The PR description should also mirror #9504's to ensure the results of this are added to the migration guide. It should also have a Co-Autthored By: line to credit @nicopap as their part of this.

@james7132 james7132 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it C-Performance A change motivated by improving speed, memory usage or compile times M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Mar 3, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 3, 2024

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@james7132 james7132 mentioned this pull request Mar 3, 2024
Closed
@alice-i-cecile alice-i-cecile removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 4, 2024
@rparrett rparrett added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@james7132 james7132 james7132 approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@nicopap nicopap nicopap approved these changes

Assignees
No one assigned
Labels
A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change C-Performance A change motivated by improving speed, memory usage or compile times M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

get_short_name should return a Cow most likely
5 participants
@tygyh @james7132 @alice-i-cecile @nicopap @rparrett