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

[Merged by Bors] - Make internal struct ShaderData non-pub #5609

Closed
wants to merge 1 commit into from

Conversation

djeedai
Copy link
Contributor

@djeedai djeedai commented Aug 7, 2022

Objective

ShaderData is marked as public, but is an internal type only used by one other
internal type, so it should be made private.

Solution

ShaderData is only used in ShaderCache, and the latter is private,
so there is no need to make the former public. This change removes the
pub keyword from ShaderData, hidding it as the implementation detail
it is.

Changelog

Removed

  • Removed ShaderData from the public API, which was only ever used internally. No public function was using it so there should be no need for any migration action.

Split from #5600

`ShaderData` is only used in `ShaderCache`, and the latter is private,
so there is no need to make the former public. This change removes the
`pub` keyword from `ShaderData`, hidding it as the implementation detail
it is.
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Aug 7, 2022
@alice-i-cecile alice-i-cecile added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Aug 7, 2022
@alice-i-cecile
Copy link
Member

Technically a breaking change, but I don't think we want or need a migration guide.

Can you add ShaderData to the PR title so the commit message is nicer.

@djeedai djeedai changed the title Make internal struct non-pub Make internal struct ShaderData non-pub Aug 7, 2022
@cart
Copy link
Member

cart commented Aug 8, 2022

bors r+

bors bot pushed a commit that referenced this pull request Aug 8, 2022
# Objective

`ShaderData` is marked as public, but is an internal type only used by one other
internal type, so it should be made private.

## Solution

`ShaderData` is only used in `ShaderCache`, and the latter is private,
so there is no need to make the former public. This change removes the
`pub` keyword from `ShaderData`, hidding it as the implementation detail
it is.

Split from #5600
@bors bors bot changed the title Make internal struct ShaderData non-pub [Merged by Bors] - Make internal struct ShaderData non-pub Aug 8, 2022
@bors bors bot closed this Aug 8, 2022
@djeedai
Copy link
Contributor Author

djeedai commented Aug 9, 2022

@alice-i-cecile we should have forced a CHANGELOG with a Migration Guide note section since this is a breaking change. Sorry I didn't think about it. Any idea if we could make that check automated?

@djeedai djeedai deleted the pub_shader_data branch August 9, 2022 08:53
maccesch pushed a commit to Synphonyte/bevy that referenced this pull request Sep 28, 2022
# Objective

`ShaderData` is marked as public, but is an internal type only used by one other
internal type, so it should be made private.

## Solution

`ShaderData` is only used in `ShaderCache`, and the latter is private,
so there is no need to make the former public. This change removes the
`pub` keyword from `ShaderData`, hidding it as the implementation detail
it is.

Split from bevyengine#5600
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

`ShaderData` is marked as public, but is an internal type only used by one other
internal type, so it should be made private.

## Solution

`ShaderData` is only used in `ShaderCache`, and the latter is private,
so there is no need to make the former public. This change removes the
`pub` keyword from `ShaderData`, hidding it as the implementation detail
it is.

Split from bevyengine#5600
@IceSentry
Copy link
Contributor

@djeedai do you think you can come up with a quick migration guide for this? You can either update the PR text or you can PR it to bevyengine/bevy-website#470

@djeedai
Copy link
Contributor Author

djeedai commented Nov 4, 2022

@IceSentry done, let me know if that's ok?

@IceSentry
Copy link
Contributor

Yep, should be good, I'll add it to the guide.

@djeedai
Copy link
Contributor Author

djeedai commented Nov 4, 2022

Ok thanks for that!

ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

`ShaderData` is marked as public, but is an internal type only used by one other
internal type, so it should be made private.

## Solution

`ShaderData` is only used in `ShaderCache`, and the latter is private,
so there is no need to make the former public. This change removes the
`pub` keyword from `ShaderData`, hidding it as the implementation detail
it is.

Split from bevyengine#5600
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants