-
Notifications
You must be signed in to change notification settings - Fork 3
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
Sync SwiftUI branch #32
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for starting to work on this @NikolaiMadlener!
I think a few things broke in the merge that would be great to be addressed (some of them are highlighted in SwiftLint, some of them approach in the build steps). Let me know of you need any support with that.
To fully support this in a Swift Package we should add it as a separate target to the current Package.swift file. We might need to have a custom source path and others in place but SPM should give us that flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this @NikolaiMadlener! This will also address #31 in the same process, I added a comment to the Package.swift file as discussed in our meeting on Monday on how we might be able to handle this in the next future in our fork.
Would be amazing if we can resolve all the open warnings & get the builds ready to eventually merge this into our main branch and setup working Swift Package SwiftUI setup 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will have to keep our old ResearchKitSwiftUI target in place while adding a new one pointing to the one created by Apple as a new target. As we will have to tag a major version anyways (moving to only iOS 18 is a breaking change), we might want to:
- Incorporate the changes here in main without adding the Apple Swift UI target and not referencing it in the Swift Package.
- Create a swiftUI branch similar to the one from Apple where we rename our old target to
ResearchKitSwitUIBridge
or something and add the new Apple target asResearchKitSwiftUI
. While Apple might take a long time to finish up their implementation we could tag4.0.0-beta.1
or similar releases on that branch including the breaking iOS 18 requirement. Eventually it would be great if Apple would do so but until then we have to maintain this ourselves ...
Sync SwiftUI branch
⚙️ Release Notes
📝 Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: