-
Notifications
You must be signed in to change notification settings - Fork 112
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 Charts from 3.6.0 to 4.0.3 and migrate to SPM #7057
Conversation
Next, we'll move it to Swift Package Manager, to make progress on running the app on Xcode 14.0. The upgrade also required to: - Rename `IAxisValueFormatter` to `AxisValueFormatter` - Update a `Description` access now that it's no longer `Optional` - Update a `.fill` assignment to match new types modeling See ChartsOrg/Charts@v3.6.0...v4.0.3
This is the only way we found to make the app build under Xcode 14.0 beta 1.
@@ -1,6 +1,6 @@ | |||
import Foundation | |||
import Charts | |||
|
|||
import UIKit |
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.
This was required for the file to compile.
I guess Charts the Package doesn't (or can't?) implicitly import UIKit like Charts the Pod did.
@@ -374,7 +374,7 @@ private extension StoreStatsV4PeriodViewController { | |||
|
|||
func configureChart() { | |||
lineChartView.marker = StoreStatsChartCircleMarker() | |||
lineChartView.chartDescription?.enabled = false | |||
lineChartView.chartDescription.enabled = false |
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.
A way to double check these changes is to open the diff between versions 3.6.0 and 4.0.3 and do a string search for the changes.
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.
Nice way for checking the changes between 2 versions 👍
You can test the changes from this Pull Request by:
|
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.
LGTM! Thanks for the updates 😍 Tested in Xcode 13.4 and the dashboard charts look as before!
@@ -374,7 +374,7 @@ private extension StoreStatsV4PeriodViewController { | |||
|
|||
func configureChart() { | |||
lineChartView.marker = StoreStatsChartCircleMarker() | |||
lineChartView.chartDescription?.enabled = false | |||
lineChartView.chartDescription.enabled = false |
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.
Nice way for checking the changes between 2 versions 👍
Description
For a reason unknown to me, Xcode 14.0 beta 1 can't compile Charts when installed via CocoaPods. That's not a big deal for us, since we plan to slowly move away from CocoaPods in favor of Swift Package Manager.
This PR does exactly that for Charts, feeding two birds with one spoon.
Testing instructions
A green CI should be enough to ensure the migration process was successful, but there could be unexpected regression as a result of bugs in the library.
I'm waiting for the installable build to verify that, butI'd appreciate other eyes on that, too.Screenshots
I don't have a store linked to my account with lots of activity. This is the only graph I could use to verify the upgrade:
RELEASE-NOTES.txt
if necessary.