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

HealthKit integration uses own contexts for CoreData #470

Merged
merged 7 commits into from
Aug 9, 2024

Conversation

theospears
Copy link
Collaborator

@theospears theospears commented Aug 9, 2024

We want to make sure HealthKit integration with CoreData is thread safe and also doesn't risk using contexts which are no longer valid. Do this by making HealthKit related code use its own contexts, and minimize how much code it holds onto.

Testing:
Ran on device
Note: Current build is known to be quite crashy

Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @theospears and the rest of your teammates on Graphite Graphite

@theospears theospears changed the title Extract monitoring code from GoalHealthKitConnection HealthKit integration uses own contexts for CoreData Aug 9, 2024
@theospears theospears marked this pull request as ready for review August 9, 2024 02:30
@theospears theospears merged commit 86eba04 into master Aug 9, 2024
3 checks passed
@theospears theospears deleted the healthkit-core-data-rules branch August 9, 2024 04:55
/// data with some lag, so we look a bit further back for safety
/// This does mean users who have very little buffer, and are not regularly unlocking their phone, may erroneously derail. There is nothing we
/// can do about this.
static let daysToUpdateOnChangeNotification = 7
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously there was a reminder about this in the form of a local notification. It was removed with #251 .

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