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 public macros more robust with $crate #4655

Closed
wants to merge 1 commit into from

Conversation

infmagic2047
Copy link
Contributor

Objective

We have some macros that are public but only used internally for now. They fail on user's code due to the use of crate names like bevy_utils, while the user only has bevy::utils. There are two affected macros.

  • bevy_utils::define_label: it may be useful in user's code for defining custom kinds of label traits (this is why I made this PR).
  • bevy_asset::load_internal_asset: not useful currently due to limitations of the debug asset server, but this may change in the future.

Solution

We can make them work by using $crate instead of names of their own crates, which can refer to the macro's defining crate regardless of the user's setup. Even though our objective is rather low-priority here, the solution adds no maintenance cost so it is still worthwhile.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label May 4, 2022
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Assets Load files from disk to use for things like images, models, and sounds A-Utils Utility functions and types and removed S-Needs-Triage This issue needs to be labelled labels May 4, 2022
@bjorn3 bjorn3 added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled and removed C-Bug An unexpected or incorrect behavior A-Assets Load files from disk to use for things like images, models, and sounds A-Utils Utility functions and types labels May 4, 2022
@bjorn3
Copy link
Contributor

bjorn3 commented May 4, 2022

Race condition. @alice-i-cecile can you restore your labels? I'm on my phone right now.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Assets Load files from disk to use for things like images, models, and sounds S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it A-Utils Utility functions and types and removed C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels May 4, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 6, 2022
# Objective

We have some macros that are public but only used internally for now. They fail on user's code due to the use of crate names like `bevy_utils`, while the user only has `bevy::utils`. There are two affected macros.

- `bevy_utils::define_label`: it may be useful in user's code for defining custom kinds of label traits (this is why I made this PR).
- `bevy_asset::load_internal_asset`: not useful currently due to limitations of the debug asset server, but this may change in the future.

## Solution

We can make them work by using `$crate` instead of names of their own crates, which can refer to the macro's defining crate regardless of the user's setup. Even though our objective is rather low-priority here, the solution adds no maintenance cost so it is still worthwhile.
@bors bors bot changed the title Make public macros more robust with $crate [Merged by Bors] - Make public macros more robust with $crate May 6, 2022
@bors bors bot closed this May 6, 2022
robtfm pushed a commit to robtfm/bevy that referenced this pull request May 10, 2022
# Objective

We have some macros that are public but only used internally for now. They fail on user's code due to the use of crate names like `bevy_utils`, while the user only has `bevy::utils`. There are two affected macros.

- `bevy_utils::define_label`: it may be useful in user's code for defining custom kinds of label traits (this is why I made this PR).
- `bevy_asset::load_internal_asset`: not useful currently due to limitations of the debug asset server, but this may change in the future.

## Solution

We can make them work by using `$crate` instead of names of their own crates, which can refer to the macro's defining crate regardless of the user's setup. Even though our objective is rather low-priority here, the solution adds no maintenance cost so it is still worthwhile.
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

We have some macros that are public but only used internally for now. They fail on user's code due to the use of crate names like `bevy_utils`, while the user only has `bevy::utils`. There are two affected macros.

- `bevy_utils::define_label`: it may be useful in user's code for defining custom kinds of label traits (this is why I made this PR).
- `bevy_asset::load_internal_asset`: not useful currently due to limitations of the debug asset server, but this may change in the future.

## Solution

We can make them work by using `$crate` instead of names of their own crates, which can refer to the macro's defining crate regardless of the user's setup. Even though our objective is rather low-priority here, the solution adds no maintenance cost so it is still worthwhile.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

We have some macros that are public but only used internally for now. They fail on user's code due to the use of crate names like `bevy_utils`, while the user only has `bevy::utils`. There are two affected macros.

- `bevy_utils::define_label`: it may be useful in user's code for defining custom kinds of label traits (this is why I made this PR).
- `bevy_asset::load_internal_asset`: not useful currently due to limitations of the debug asset server, but this may change in the future.

## Solution

We can make them work by using `$crate` instead of names of their own crates, which can refer to the macro's defining crate regardless of the user's setup. Even though our objective is rather low-priority here, the solution adds no maintenance cost so it is still worthwhile.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Utils Utility functions and types C-Bug An unexpected or incorrect behavior 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.

3 participants