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

feat: Meet api interoperability guidelines #4

Merged
merged 2 commits into from
May 23, 2024

Conversation

Zoybean
Copy link
Contributor

@Zoybean Zoybean commented May 23, 2024

interoperability guidelines: https://rust-lang.github.io/api-guidelines/interoperability.html

I've added derives for Eq, Copy, and Clone where possible, and for Ord where it didn't seem misleading (i.e. no Ord on Point because it's 2d, but trivial Ord on ConstCap).
I added derives for Hash for Point and Boundary, which I thought could hypothetically be used as sensible keys for some structure, and for the capacity types, which could hypothetically be nested within a sensible key, but not e.g. QuadTree, the iterators, or the error.

Zoybean added 2 commits May 23, 2024 15:23
Interoperability guidelines: https://rust-lang.github.io/api-guidelines/interoperability.html

Add derives for `Eq`, `Copy`, and `Clone` where possible (but no `Copy` + `Iterator`)
Add derives for `Ord` where it does't seem misleading (i.e. no `Ord` on `Point` because it's 2d, but trivial `Ord` on `ConstCap`).
Add derives for `Hash`:
- for `Point` and `Boundary`, which could hypothetically be used as sensible keys
- for the capacity types, which could hypothetically be nested within a sensible key type
- not for `QuadTree`, the iterators, or the error
@Julian-Alberts Julian-Alberts merged commit 75eaa93 into Julian-Alberts:main May 23, 2024
@Julian-Alberts
Copy link
Owner

@Zoybean thanks for the contribution.

@Zoybean Zoybean deleted the push-tmzyxoovpvns branch May 23, 2024 23:48
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