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

[gardening]: remove unneeded code from ChildKey #324

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

jamieQ
Copy link
Contributor

@jamieQ jamieQ commented Mar 11, 2025

small refactor of ChildKey to remove custom Hashable requirement implementations in favor of synthesized ones

Comment on lines -415 to -423
func hash(into hasher: inout Hasher) {
hasher.combine(ObjectIdentifier(childType))
hasher.combine(key)
}

static func == (lhs: ChildKey, rhs: ChildKey) -> Bool {
lhs.childType == rhs.childType
&& lhs.key == rhs.key
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we get these for free if all our stored properties are Hashable

Copy link
Collaborator

Choose a reason for hiding this comment

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

you may find this wrapper useful to do the same thing with better debugging/logging

@jamieQ jamieQ marked this pull request as ready for review March 12, 2025 03:04
@jamieQ jamieQ requested a review from a team as a code owner March 12, 2025 03:04
Copy link

@thingdeux thingdeux left a comment

Choose a reason for hiding this comment

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

Makes sense 🤙🏽 - LGTM

Comment on lines -415 to -423
func hash(into hasher: inout Hasher) {
hasher.combine(ObjectIdentifier(childType))
hasher.combine(key)
}

static func == (lhs: ChildKey, rhs: ChildKey) -> Bool {
lhs.childType == rhs.childType
&& lhs.key == rhs.key
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

you may find this wrapper useful to do the same thing with better debugging/logging

@jamieQ jamieQ merged commit c98a83a into main Mar 12, 2025
7 checks passed
@jamieQ jamieQ deleted the jquadri/almost-spring-gardening branch March 12, 2025 18:09
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.

5 participants