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] - [bevy_derive] Refactor modules for better error message. #2059

Conversation

NathanSWard
Copy link
Contributor

Problem:

  • When using the 'as_crate' attribute, if 'as_crate' was empty, the only
    error you would get is 'integer underflow'.

Solution:

  • Provide an explicit check for the 'as_crate' attribute's token stream
    to ensure the formatting is correct.

Note:

  • Also reworked 'get_meta' by not making it call 'Manifest::find' twice.

@NathanSWard NathanSWard force-pushed the nward/bevy-derive-modules-refactor branch 2 times, most recently from daa36ff to 866fa62 Compare April 30, 2021 19:41
@alice-i-cecile alice-i-cecile added the C-Code-Quality A section of code that is hard to understand or change label Apr 30, 2021
}
Manifest::new()
.unwrap()
.find(|name| name == "bevy" || name == "bevy_internal")
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes the output of this function depend on the order of dependencies if both bevy and bevy_internal are specified as dependencies for some reason.

Copy link
Contributor Author

@NathanSWard NathanSWard Apr 30, 2021

Choose a reason for hiding this comment

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

yes true, though this was the previous behavior of the implementation IIUC?

Edit: oh, now I understand what you mean!

Problem:
- When using the 'as_crate' attribute, if 'as_crate' was empty, the only
  error you would get is 'integer underflow'.

Solution:
- Provide an explicit check for the 'as_crate' attribute's token stream
  to ensure the formatting is correct.
@NathanSWard NathanSWard force-pushed the nward/bevy-derive-modules-refactor branch from 866fa62 to 7d41785 Compare May 1, 2021 03:41
@alice-i-cecile alice-i-cecile added 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 May 1, 2021
@cart
Copy link
Member

cart commented May 1, 2021

As a heads up: this will conflict with #1875.

@cart
Copy link
Member

cart commented May 6, 2021

Merging this because its ready now :)

@cart
Copy link
Member

cart commented May 6, 2021

bors r+

bors bot pushed a commit that referenced this pull request May 6, 2021
Problem:
- When using the 'as_crate' attribute, if 'as_crate' was empty, the only
  error you would get is 'integer underflow'.

Solution:
- Provide an explicit check for the 'as_crate' attribute's token stream
  to ensure the formatting is correct.

Note:
- Also reworked 'get_meta' by not making it call 'Manifest::find' twice.
@bors bors bot changed the title [bevy_derive] Refactor modules for better error message. [Merged by Bors] - [bevy_derive] Refactor modules for better error message. May 7, 2021
@bors bors bot closed this May 7, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
…2059)

Problem:
- When using the 'as_crate' attribute, if 'as_crate' was empty, the only
  error you would get is 'integer underflow'.

Solution:
- Provide an explicit check for the 'as_crate' attribute's token stream
  to ensure the formatting is correct.

Note:
- Also reworked 'get_meta' by not making it call 'Manifest::find' twice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants