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

Fix bug with setting options on LocationManager #992

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

macdrevx
Copy link
Contributor

@macdrevx macdrevx commented Jan 5, 2022

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
  • Update the changelog

Summary of changes

LocationProvider is not constrained to be a class, so when syncOptions() was called, it triggered the setter for the property that stored it rather than merely setting a property on the object itself. This caused the location provider to be stopped and restarted and have its delegate reconfigured each time options were set on LocationManager.

In v11, let's plan to constrain LocationProvider to AnyObject and remove this workaround.

@macdrevx macdrevx added the bug 🪲 Something is broken! label Jan 5, 2022
LocationProvider is not constrained to be a class, so when syncOptions()
was called, it triggered the setter for the property that stored it
rather than merely setting a property on the object itself. This caused
the location provider to be stopped and restarted and have its delegate
reconfigured each time options were set on LocationManager.
@macdrevx macdrevx force-pushed the fix/location-provider-did-set branch from 31d9fff to dcf7e64 Compare January 5, 2022 21:38
@macdrevx macdrevx enabled auto-merge (squash) January 5, 2022 21:44
@macdrevx macdrevx merged commit 4e698d8 into main Jan 5, 2022
@macdrevx macdrevx deleted the fix/location-provider-did-set branch January 5, 2022 21:44
OdNairy pushed a commit that referenced this pull request Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something is broken!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants