Update to Swift 4.2, Adopt Hasher #164
Conversation
|
Hmm.. also not sure why the checks are failing. On my mac, everything builds properly. Is it due to the fact that I updated it to Swift 4.2 and the checks are configured for Swift 4.0? |
|
It's because CI is on a Xcode 9 stack and Swift 4.0. |
|
Great! I'll fix this when I get home. |
|
How does this look @frederoni? |
|
Sorry about the delay here. |
Sync changes
|
Done! |
|
I forgot to enable forked PRs. 🙈 One more time. Just an empty commit or rebase. |
|
No worries! Just to make sure is that what I was supposed to do? |
|
It was supposed to start the CI bots but it looks like it didn't trigger. Just changing the target branch doesn't work. You need to amend an empty commit to change the commit SHA1 or rebase the PR. |
|
I made a bogus commit. |
|
It builds now! |
|
Ok, so essentially this latest commit uses the synthesized hash if the swift version > 4.2 but it uses the old computed version if < 4.2. |
|
Looking good 👍 |
|
Sweet! The only thing though is by using the synthesized hash, some properties that aren't necessary might be used in calculating the hash and explicitly declaring our own hash gets rid of that possibility and uses just the coordinates. It's not a major issue as it should be calculated just fine either way, but not sure if it made too much of a difference. |
|
I think it's a good thing that the hash represents all properties including the coordinates and not just the identifier here. |
|
Yeah, you have a point. |
|
@1ec5, just to make sure you haven't missed the notification. Do you think you could check this out really quickly? |
|
Haha no worries! |
This PR updates the MapboxGeocoder to support Swift 4.2. Additionally, it adopts Hasher and resolves #146.