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

Update to SpeziScheduler 1.0 #84

Merged
merged 12 commits into from
Nov 10, 2024
Merged

Update to SpeziScheduler 1.0 #84

merged 12 commits into from
Nov 10, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Aug 29, 2024

Update to SpeziScheduler 1.0

♻️ Current situation & Problem

This PR updates the SpeziTemplateApplication to use the new SpeziScheduler 1.0 release. This allow use to remove a lot of code that was previously necessary in the template application. Generally, we were able to greatly simplify the Scheduler setup.

⚙️ Release Notes

  • Upgrade SpeziScheduler to 1.0

📚 Documentation

We updated the documentation to reflect the changes done to the template app.

✅ Testing

Existing UI test cases are reused.

📝 Code of Conduct & Contributing Guidelines

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

@Supereg Supereg force-pushed the feature/infinity-loop-scheduler branch from d4d8988 to 847f397 Compare September 3, 2024 08:44
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.

Very cool to see the simplifications in here!

TemplateApplication.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
TemplateApplication.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
TemplateApplication.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
TemplateApplication/Home.swift Outdated Show resolved Hide resolved
TemplateApplication/Schedule/ScheduleView.swift Outdated Show resolved Hide resolved
@PSchmiedmayer PSchmiedmayer added the enhancement New feature or request label Oct 21, 2024
@Supereg Supereg changed the title Update to new Scheduler Update to SpeziScheduler 1.0 Oct 29, 2024
@Supereg Supereg marked this pull request as ready for review October 29, 2024 15:16
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.

Thank you for working on this; happy to see this merged once we move to fully tagged version in the template app; we should also move Foundation, Accounts, and others to the tagged versions that are not betas 👍

@Supereg
Copy link
Member Author

Supereg commented Oct 29, 2024

@PSchmiedmayer it seems like periphery doesn’t support swift 6? Or do you have a clue what the issue could be with the failing build?

@PSchmiedmayer
Copy link
Member

Seem like this might be resolved soon: peripheryapp/periphery#813

Wondering we should temporarily remove CodeQL and Periphery and add an issue to re-add them once they support Xcode 16/Swift 6?

Or maybe make them non-required to merge a PR?

@Supereg
Copy link
Member Author

Supereg commented Oct 30, 2024

Seem like this might be resolved soon: peripheryapp/periphery#813

Wondering we should temporarily remove CodeQL and Periphery and add an issue to re-add them once they support Xcode 16/Swift 6?

Or maybe make them non-required to merge a PR?

I think making Peripheral optional makes sense. They seem to be working on this. Not sure how long it takes for CodeQL to support Swift 6.

Other than that, this PR is ready to be merged 👍

@PSchmiedmayer
Copy link
Member

PSchmiedmayer commented Oct 30, 2024

Good point; I think we can make both optional for now but keep the actions in there.

I have asked for an estimate on the CodeQL Swift 6 adoption in their GitHub repo: github/codeql-action#2506.
There seems to be some ongoing work, I am sure they are working on supporting this in the near future: github/codeql#17699

It seems like some of the UI tests are still failing at this point; I would be fine with going ahead and merging the PR with CodeQL and Periphery failing for now (they will work again once support is added to these tools).

@Supereg
Copy link
Member Author

Supereg commented Oct 31, 2024

Good point; I think we can make both optional for now but keep the actions in there.

I have asked for an estimate on the CodeQL Swift 6 adoption in their GitHub repo: github/codeql-action#2506. There seems to be some ongoing work, I am sure they are working on supporting this in the near future: github/codeql#17699

It seems like some of the UI tests are still failing at this point; I would be fine with going ahead and merging the PR with CodeQL and Periphery failing for now (they will work again once support is added to these tools).

Just found, that Periphery only fails because of the project format being Xcode 16.0. I just reverted to the Xcode 15.3 project format as it isn't really important and periphery works again. Now we are just waiting for CodeQL.

Another thing I noticed was that the Build and Test currently always choses the iPhone SE which makes some of our unit test fail as we would need to add additional behavior for scrolling. I tried to change the device to iPhone 16 Pro in Fastlane, however that didn't do anything. Do you know what the issue might be?

@PSchmiedmayer
Copy link
Member

Very cool; that's great regarding Periphery!

Strange thing regarding the simulator; must be related to what simulators are installed on the runners. I thought all of them should be installed, but there might be a mismatch ...
I will take a look at this 👍

@PSchmiedmayer
Copy link
Member

@Supereg I updated some of our agents and the builds seem to run now; just seems to be a UI testing issue with our license test that would stop us from merging the PR 👍

@Supereg
Copy link
Member Author

Supereg commented Nov 9, 2024

@Supereg I updated some of our agents and the builds seem to run now; just seems to be a UI testing issue with our license test that would stop us from merging the PR 👍

Great news. I’ll make sure to resolve that 👍

@PSchmiedmayer
Copy link
Member

Thank you 🎉

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

Attention: Patch coverage is 83.65385% with 17 lines in your changes missing coverage. Please review.

Project coverage is 86.92%. Comparing base (bb810ce) to head (af6803f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
TemplateApplication/Schedule/EventView.swift 53.34% 14 Missing ⚠️
TemplateApplication/Account/AccountSheet.swift 50.00% 1 Missing ⚠️
...cation/Schedule/TemplateApplicationScheduler.swift 93.34% 1 Missing ⚠️
...plateApplication/TemplateApplicationStandard.swift 66.67% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
+ Coverage   81.44%   86.92%   +5.48%     
==========================================
  Files          24       21       -3     
  Lines         792      688     -104     
==========================================
- Hits          645      598      -47     
+ Misses        147       90      -57     
Files with missing lines Coverage Δ
TemplateApplication/Account/AccountButton.swift 100.00% <100.00%> (ø)
...mplateApplication/Account/AccountSetupHeader.swift 100.00% <100.00%> (ø)
TemplateApplication/Contacts/Contacts.swift 91.84% <100.00%> (ø)
TemplateApplication/HomeView.swift 100.00% <100.00%> (ø)
...lateApplication/Onboarding/AccountOnboarding.swift 100.00% <100.00%> (ø)
...eApplication/Onboarding/HealthKitPermissions.swift 95.56% <100.00%> (ø)
...ateApplication/Onboarding/InterestingModules.swift 100.00% <100.00%> (ø)
...plication/Onboarding/NotificationPermissions.swift 95.56% <100.00%> (ø)
...emplateApplication/Onboarding/OnboardingFlow.swift 97.50% <100.00%> (+0.28%) ⬆️
TemplateApplication/Onboarding/Welcome.swift 100.00% <100.00%> (ø)
... and 6 more

... and 2 files with indirect coverage changes


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 bb810ce...af6803f. Read the comment docs.

@Supereg
Copy link
Member Author

Supereg commented Nov 10, 2024

Thank you 🎉

Apparently, this was just a Springboard Reboot in the Simulator while executing this test case. Everything passes now.

@Supereg Supereg merged commit fed2d72 into main Nov 10, 2024
10 of 11 checks passed
@Supereg Supereg deleted the feature/infinity-loop-scheduler branch November 10, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants