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

Fix #773 #774

Merged
merged 3 commits into from
Apr 24, 2024
Merged

Fix #773 #774

merged 3 commits into from
Apr 24, 2024

Conversation

ClaasJG
Copy link
Contributor

@ClaasJG ClaasJG commented Apr 22, 2024

Hi,

This fixes #773 .

Checklist

  • I've added tests for all code changes and additions (where applicable)
  • I've added a demonstration of the new feature to one or more examples
  • I've updated the book to reflect my changes
  • Usage of new public items is shown in the API docs

I thought this was already possible based on the resources mentioned in the list above.
Therefore, I have not changed anything and don't think anything needs to be added / changed.

-ClaasJG

Copy link
Contributor

@Imberflur Imberflur left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for this improvement, it looks great!

I left one suggestion for improving the docs.

specs-derive/src/lib.rs Show resolved Hide resolved
specs-derive/src/lib.rs Outdated Show resolved Hide resolved
@ClaasJG
Copy link
Contributor Author

ClaasJG commented Apr 23, 2024

I added an explanation, that #[storage(...)] accepts storage types with type parameters.
English is not my first language and the text feels clunky to me.

@ClaasJG
Copy link
Contributor Author

ClaasJG commented Apr 23, 2024

Apparently I required one more , for cargo fmt to be happy. I squashed the new format into the first fixing commit which introduced the regression and force pushed to maintain the 3 'clean' commits.

I apologise for the trouble and thank you for your patience with me.

@ClaasJG
Copy link
Contributor Author

ClaasJG commented Apr 23, 2024

I dont know how to fix the failing check on nightly, nor am I sure, it is caused by this pull request.

@Imberflur
Copy link
Contributor

I dont know how to fix the failing check on nightly, nor am I sure, it is caused by this pull request.

No worries, it isn't caused by this PR.

I'm planning to do some quick testing to confirm works with the new derive logic, and will merge this after I do that.

@Imberflur Imberflur merged commit b0c961d into amethyst:master Apr 24, 2024
12 of 19 checks passed
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.

specs-derive better 'storage' macro parameter
2 participants