[breaking change] update to new SpeziStudy release#34
Conversation
github-project-automation
Bot
moved this to In Progress
in MyHeart Counts Project Planning
📝 WalkthroughWalkthroughPackage.swift updates SwiftPM dependencies (SpeziStudy to a commit revision, SpeziFoundation to 2.7.7) and removes MainActor isolation settings from both targets. Study.swift imports SpeziLocalization and refactors metadata to use localized dictionaries, increments study revision to 41, and removes the enrollmentConditions field. ChangesStudy Definition Localization and Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/MHCStudyDefinitionExporter/Study.swift (1)
45-51: ⚡ Quick winLocalization fallback for UK (en-GB) users
SpeziLocalizationfalls back across English region variants, so UK users should still receive the shared English copy even thoughtitle,shortTitle, andshortExplanationTextare keyed only by.enUS(inSources/MHCStudyDefinitionExporter/Study.swift:45-51). No coverage must-fix needed; using a language-neutral English key would just be a stylistic improvement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/MHCStudyDefinitionExporter/Study.swift` around lines 45 - 51, The localization entries for title, shortTitle, and shortExplanationText use the region-specific key .enUS which relies on fallback behavior for UK users; change these to the language-neutral English key (e.g., use the non-region English localization key your localization API provides) so title, shortTitle, and shortExplanationText are keyed by plain English rather than .enUS (update the mappings in the Study struct where title, shortTitle, and shortExplanationText are defined).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Package.swift`:
- Around line 30-32: The Package.swift currently pins SpeziStudy using a
specific commit via .package(..., revision: "e11803b...") which prevents
downstream coordinated version resolution; update the SpeziStudy dependency
declaration (the .package entry for "SpeziStudy") to reference a released
version instead (use a semantic version requirement such as from: "X.Y.Z" or
.exact("X.Y.Z") / .upToNextMajor(from: "X.Y.Z") matching your coordinated
release) so SwiftPM can resolve versions correctly for consumers.
---
Nitpick comments:
In `@Sources/MHCStudyDefinitionExporter/Study.swift`:
- Around line 45-51: The localization entries for title, shortTitle, and
shortExplanationText use the region-specific key .enUS which relies on fallback
behavior for UK users; change these to the language-neutral English key (e.g.,
use the non-region English localization key your localization API provides) so
title, shortTitle, and shortExplanationText are keyed by plain English rather
than .enUS (update the mappings in the Study struct where title, shortTitle, and
shortExplanationText are defined).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f19cb429-556d-492c-861c-b1999db8bdd3
📒 Files selected for processing (2)
Package.swiftSources/MHCStudyDefinitionExporter/Study.swift
| .package(url: "https://github.com/StanfordSpezi/SpeziStudy.git", revision: "e11803b1897930da7c374085606814e82622bdf3"), | ||
| .package(url: "https://github.com/StanfordSpezi/SpeziFoundation.git", from: "2.7.7"), | ||
| .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.6.2") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Swift Package Manager allow a package that is itself published/tagged for versioned consumption to depend on another package via .revision(...), or are revision-based requirements only supported in the root package?
💡 Result:
Yes—any package’s manifest can express a revision-based dependency, not only the root package. In SwiftPM, a dependency requirement can be a Git revision via .revision(_:) (or by using package(url:revision:)). This is encoded in the package’s Package.Dependency.Requirement API and is used when resolving remote dependencies, regardless of whether that manifest belongs to the root package or a dependency in the graph. [1][2][3] Important nuance: if your published/tagged package uses commit-based requirements (including .revision), it cannot be depended upon by packages that use version-based dependency requirements (so you may need to avoid commit-based requirements when you intend others to use version constraints against your package). [1][2] Example from the manifest API: dependencies: [ .package(url: "https://example.com/example-package.git", revision: "<40-char-commit-hash>") ] [1][3]
Citations:
- 1: https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/PackageDescription.md
- 2: https://developer.apple.com/documentation/packagedescription/package/dependency/requirement-swift.enum/revision(_:)
- 3: https://developer.apple.com/documentation/packagedescription/package/dependency/package(url:revision:)
Avoid pinning SpeziStudy with .revision(...) if you’re tagging a coordinated release—prefer the released tag/exact version instead.
SwiftPM allows revision-based dependency requirements in any package manifest, so this won’t inherently prevent versioned consumers from resolving your package. Still, pinning SpeziStudy to a specific commit (as in Package.swift:30-32) removes downstream flexibility to use SpeziStudy version constraints for coordinated updates; using a released tag/exact version is safer for release hygiene.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Package.swift` around lines 30 - 32, The Package.swift currently pins
SpeziStudy using a specific commit via .package(..., revision: "e11803b...")
which prevents downstream coordinated version resolution; update the SpeziStudy
dependency declaration (the .package entry for "SpeziStudy") to reference a
released version instead (use a semantic version requirement such as from:
"X.Y.Z" or .exact("X.Y.Z") / .upToNextMajor(from: "X.Y.Z") matching your
coordinated release) so SwiftPM can resolve versions correctly for consumers.
| .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.6.2"), | ||
| // not used directly but we need to fix it below 0.9.0 for the time being | ||
| .package(url: "https://github.com/apple/FHIRModels.git", .upToNextMinor(from: "0.8.0")) | ||
| .package(url: "https://github.com/StanfordSpezi/SpeziStudy.git", revision: "e11803b1897930da7c374085606814e82622bdf3"), |
There was a problem hiding this comment.
TODO to add a tagged version here.
|
@lukaskollmer I no longer have owner permissions for the @StanfordBDHG org to either move this repo as planned or add the self-hosted runners temporarily to the org to merge the PR; sorry for the delay here! I will let you know once I get any updates. |
|
superseded by #35 |
github-project-automation
Bot
moved this from In Progress
to Done
in MyHeart Counts Project Planning
github-project-automation
Bot
moved this from In Progress
to Done
in MyHeart Counts Project Planning
2 participants
[breaking change] update to new SpeziStudy release
Note this is a breaking change so we'll need to coordinate the release with a corresponding MHC app release
⚙️ Release Notes
📚 Documentation
n/a
✅ Testing
n/a
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: