Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

use gyb to define well-known sample types #30

Merged
merged 3 commits into from
Feb 2, 2025

Conversation

lukaskollmer
Copy link
Member

@lukaskollmer lukaskollmer commented Jan 29, 2025

use gyb to define well-known sample types

♻️ Current situation & Problem

SpeziHealthKit currently has a hardcoded set of extensions on the SampleType struct, which define well-known sample types.
This is not easy to maintain, especially if we want to add additional features in the future which should also work on all of these well-known sample types (we do want to do this).
For example, it will at some point become necessary to have the ability to treat SampleTypes as codable-like objects, in which case we'd need the ability to select a specific well-known sample type, based on its underlying HealthKit identifier.
Using the current approach, this is of course possible but it would be rather annoying to implement, and very easy to mess up.

Using GYB instead allows us to have the SampleType extensions be created in an automated and well-defined way, and it's a lot easier to add new sample types, or new functionality that needs to be implemented in some way that is aware of all sample types.

⚙️ Release Notes

  • added:
    • SampleType<HKQuantitySample>(identifier: HKQuantityTypeIdentifier)
    • SampleType<HKCorrelation>(identifier: HKCorrelationTypeIdentifier)
    • SampleType<HKCategorySample>(identifier: HKCategoryIdentifier)

(the gyb change is an implementation detail and doesn't affect the public API of the package.

📚 Documentation

unchanged

✅ Testing

unchanged

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@lukaskollmer
Copy link
Member Author

@PSchmiedmayer what's your take on this?

On one hand, this kinda introduces a new dependency into the codebase (GYB, though it's really only needed on the package-developer-side, when having changed the list of well-known sample types and wanting to regenerate the file), but on the other hand, it does allow for us to easily support some things we currently would need to implement by hand, and which would potentially be cumbersome to maintain (i.e., easy to accidentally mess up).

I thought about whether this could be done using Swift Macros, but AFAIK the answer would be no, since (if we had e.g. an @DefineSampleType(...) macro) in addition to rewriting the macro into a static computed property, we'd also need to somehow keep track of all sample types, and their respective generic bounds, so that we could use them in e.g. a hypothetical init(identifier:).

Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.61%. Comparing base (1650f93) to head (3441a1a).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #30   +/-   ##
=======================================
  Coverage   83.61%   83.61%           
=======================================
  Files          42       42           
  Lines        2372     2372           
=======================================
  Hits         1983     1983           
  Misses        389      389           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1650f93...3441a1a. Read the comment docs.

@PSchmiedmayer PSchmiedmayer added the enhancement New feature or request label Jan 29, 2025
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Nice, thank you for setting this up; looks great!

I am find with using it here, purely developer focused and someone who wants to check out the repo doesn't even have to use it as the generated file is part of the repo; good middle ground.

Copy link
Member

@philippzagar philippzagar left a comment

Choose a reason for hiding this comment

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

Great work!

@lukaskollmer lukaskollmer merged commit 121a054 into main Feb 2, 2025
8 of 9 checks passed
@lukaskollmer lukaskollmer deleted the lukas/gyb-sample-types branch February 2, 2025 14:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants