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

Expose the wrapped storage from Read/WriteStorage #419

Merged
merged 3 commits into from
May 19, 2018
Merged

Expose the wrapped storage from Read/WriteStorage #419

merged 3 commits into from
May 19, 2018

Conversation

andrewhickman
Copy link
Contributor

@andrewhickman andrewhickman commented May 19, 2018

This is already possible through Join::open (although not very ergonomic), so there shouldn't be any soundness issues here. This is useful for using custom storages with additional methods.


This change is Reviewable

Copy link
Member

@torkleyy torkleyy left a comment

Choose a reason for hiding this comment

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

Thanks, just one small concern about the naming.

@@ -192,6 +192,11 @@ where
T: Component,
D: Deref<Target = MaskedStorage<T>>,
{
/// Gets the wrapped storage.
pub fn storage(&self) -> &T::Storage {
Copy link
Member

Choose a reason for hiding this comment

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

Could we name this unprotected_storage? I usually prefer short names, but in this case it's not what most people are looking to use, so I think it's good to have some kind of warning.

@torkleyy torkleyy added the ready label May 19, 2018
@torkleyy
Copy link
Member

Ah please also do a bump of the patch version so I can release after it's merged.

@torkleyy
Copy link
Member

Okay, this time my review comes in three parts, I have one last request: Please add your change to the changelog. That should be it now :)

Copy link
Member

@torkleyy torkleyy left a comment

Choose a reason for hiding this comment

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

Thank you so much @andrewhickman!

bors r+

bors bot added a commit that referenced this pull request May 19, 2018
419: Expose the wrapped storage from Read/WriteStorage r=torkleyy a=andrewhickman

This is already possible through `Join::open` (although not very ergonomic), so there shouldn't be any soundness issues here. This is useful for using custom storages with additional methods.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/slide-rs/specs/419)
<!-- Reviewable:end -->


Co-authored-by: Andrew Hickman <andrew.hickman1@sky.com>
@bors
Copy link
Contributor

bors bot commented May 19, 2018

Build succeeded

@bors bors bot merged commit 9232fa5 into amethyst:master May 19, 2018
@torkleyy
Copy link
Member

Specs 0.11.2 is uploaded now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants