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

Remove :: prefix from derived implementation. #103

Merged
merged 3 commits into from
Jan 3, 2019

Conversation

azriel91
Copy link
Member

@azriel91 azriel91 commented Dec 6, 2018

This allows an in-scope re-exported shred crate to be used.

Closes #102


I'll test this in a larger project, give me a few days to get to it.

This allows an in-scope re-exported `shred` crate to be used.

Issue amethyst#102
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.

Can we do this conditionally based on the edition being used? I don't want to break compatibility yet.

@azriel91
Copy link
Member Author

Hm, the changes were more of a "get the user to explicitly use shred" instead of a Rust 2018 change. Would you still like it to be behind a feature gate? (can't find a way to detect based on edition)

@torkleyy
Copy link
Member

torkleyy commented Dec 11, 2018 via email

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.

Another question is whether shred or the individual symbols should be used. For other macros the latter is the case by now.

@azriel91
Copy link
Member Author

azriel91 commented Dec 23, 2018

that would make it a breaking change

yeap, would mean shred-derive 0.6; shred itself doesn't change so can stay at 0.7.1

whether shred or the individual symbols should be used

Going by the other crates, I'd go with the individual symbols for consistency. Reminder for myself: even if shred was re-exported under a different name, you can still import it as shred.

This means users have to explicitly import the required types and
traits.

Issue amethyst#102
@azriel91
Copy link
Member Author

azriel91 commented Dec 23, 2018

Pushed a change for the latter method, people would need to make sure shred::{ResourceId, Resources, SystemData} are in scope.

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!
bors r+

bors bot added a commit that referenced this pull request Jan 3, 2019
103: Remove `::` prefix from derived implementation. r=torkleyy a=azriel91

This allows an in-scope re-exported `shred` crate to be used.

Closes #102

---

I'll test this in a larger project, give me a few days to get to it.

Co-authored-by: Azriel Hoh <azriel91@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jan 3, 2019

Build succeeded

@bors bors bot merged commit 21d1051 into amethyst:master Jan 3, 2019
@azriel91 azriel91 deleted the issue/102/use-in-scope-shred branch January 26, 2019 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants