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

Backport HealthKit fix to earlier version #22

Merged
merged 4 commits into from
Apr 5, 2024
Merged

Conversation

AndreasStokholm
Copy link
Contributor

I originally implemented this fix in a slightly newer version, so this is a backport. This meant that I had to make a few changes in NS and some other places to make the API compatible. The actual fix itself is what's in HealthKitManager.swift, everything else is to conform to that version of the HKM API wherever it's being called.

More specifically, the HealthKitManager fix are the following parts:

@AndreasStokholm AndreasStokholm changed the title Backport healthkit fix to earlier version Backport HealthKit fix to earlier version Mar 20, 2024
Copy link
Contributor

@bastiaanv bastiaanv left a comment

Choose a reason for hiding this comment

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

Haven't run the code, but looks good!

FreeAPS/Sources/Services/HealthKit/HealthKitManager.swift Outdated Show resolved Hide resolved
@marionbarker
Copy link
Contributor

In code review:

  • I noticed that in several places - there is a change from "FreeAPS" (root string) to "iAPS"
  • Some comments but some in code

Will this be changed to Open-iAPS?

@dnzxy
Copy link
Contributor

dnzxy commented Mar 30, 2024

Will this be changed to Open-iAPS?

Should be, yes. Those string changes were something I did in a very early version before Andreas‘ rewrite, where I had added more logging and the app was named iAPS back then 😂

@bjornoleh
Copy link
Contributor

Will this be changed to Open-iAPS?

Should be, yes. Those string changes were something I did in a very early version before Andreas‘ rewrite, where I had added more logging and the app was named iAPS back then 😂

I took the liberty of replacing iAPS with Open-iAPS in comments of HealthKitManager.swift :-)

Anything else missing now?

@AndreasStokholm, there is a positive review by Bastiaan above, with a minor comment about debug vs print.

@AndreasStokholm AndreasStokholm merged commit 1e8c954 into dev Apr 5, 2024
bjornoleh added a commit that referenced this pull request Apr 6, 2024
bjornoleh added a commit to bjornoleh/old_trio that referenced this pull request Apr 6, 2024
bjornoleh added a commit that referenced this pull request Apr 11, 2024
Remove `actualDate` as introduced in #22
@MikePlante1 MikePlante1 mentioned this pull request Apr 11, 2024
4 tasks
@MikePlante1 MikePlante1 deleted the apple-health-fix branch May 4, 2024 02:21
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.

None yet

5 participants