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.

Sign up for GitHub

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

Jump to bottom

ahash re-export might not be needed #3482

Closed
stefnotch opened this issue Oct 17, 2023 · 0 comments · Fixed by #4979
Closed

ahash re-export might not be needed #3482

stefnotch opened this issue Oct 17, 2023 · 0 comments · Fixed by #4979

Comments

@stefnotch
Copy link

Is your feature request related to a problem? Please describe.
e-gui reexports ahash. This doesn't seem necessary anymore, since none of the egui public APIs use an ahash type.

Whenever I use egui in an IDE and import HashMap, then Rust Analyzer will rightfully ask "std or ahash". And it'll sort them alphabetically, so it'll actually suggest the ahash one first.

Describe the solution you'd like
If possible, it'd be nice if egui didn't reexport ahash. Then I'd no longer get bonus import options that I don't necessarily want.
@oscargus oscargus mentioned this issue May 17, 2024
Merged
emilk pushed a commit that referenced this issue May 27, 2024
@oscargus
Do no use the ahash reimport (#4504)
cd45d18 
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

Related to #3482 

Not sure what the "best practice" is, to me it seems like one should
import from "the original location" if possible, but now it should at
least be possible to not re-export ahash without any breakage in the
egui code base (but possibly in projects using egui, so one should
probably deprecate it if one would like to go that path). It also seems
like epaint re-exports ahash.
@oscargus oscargus mentioned this issue Aug 18, 2024
Merged
1 task
emilk pushed a commit that referenced this issue Aug 26, 2024
@oscargus
Deprecate ahash re-exports (#4979)
cc3a091 
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->
* Closes <#3482>
* [x] I have followed the instructions in the PR template

Regarding the statement in #3482,
https://docs.rs/egui/latest/egui/layers/struct.GraphicLayers.html#method.drain
accepts a HashMap, but not sure if that is enough to keep the re-export.
486c pushed a commit to 486c/egui that referenced this issue Oct 9, 2024
@oscargus @486c
Deprecate ahash re-exports (emilk#4979)
0c50252 
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->
* Closes <emilk#3482>
* [x] I have followed the instructions in the PR template

Regarding the statement in emilk#3482,
https://docs.rs/egui/latest/egui/layers/struct.GraphicLayers.html#method.drain
accepts a HashMap, but not sure if that is enough to keep the re-export.
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
@oscargus @hacknus
Do no use the ahash reimport (emilk#4504)
1fc89e6 
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

Related to emilk#3482 

Not sure what the "best practice" is, to me it seems like one should
import from "the original location" if possible, but now it should at
least be possible to not re-export ahash without any breakage in the
egui code base (but possibly in projects using egui, so one should
probably deprecate it if one would like to go that path). It also seems
like epaint re-exports ahash.
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
@oscargus @hacknus
Deprecate ahash re-exports (emilk#4979)
8bbeef6 
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->
* Closes <emilk#3482>
* [x] I have followed the instructions in the PR template

Regarding the statement in emilk#3482,
https://docs.rs/egui/latest/egui/layers/struct.GraphicLayers.html#method.drain
accepts a HashMap, but not sure if that is enough to keep the re-export.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate ahash re-exports oscargus/egui
1 participant
@stefnotch