Clean build on Swift 3.1 #419
Conversation
Using two forks temporarily. Using bharat/SwiftCharts because we need the extension safe commits from master, but don't want the rest of the master branch yet. Using bharat/LoopKit because we want the warning fixes from LoopKit/LoopKit#111 which is not yet merged into LoopKit/dev
|
|
|
I kicked the build, and I think it passed? But it's not showing up as passing here yet for some reason. |
| return [ | ||
| "## ShareClientManager", | ||
| "latestBackfill: \(latestBackfill)", | ||
| "latestBackfill: \(latestBackfill.debugDescription)", |
There was a problem hiding this comment.
It's preferable to call String(reflecting: x) vs calling .debugDescription directly because that works with any instances, including those cases when the runtime will synthesize the string representation for you.
|
The frameworks in here need to be built from a LoopKit release tag. |
1. Change all explicit calls to .debugDescription over to String(describing:). This preserves existing behavior which was happening implicitly before swift 3.1 2. Use CGFloat.pi instead of CGFloat(Double.pi), and fix an issue where I accidentally negated one of the values in an earlier commit
Fix Swift 3.1 warnings from G4ShareSpy, ShareClient
|
@ps2 when you say happy to do that, but I wasn't sure which release tag for LoopKit would cover this commit: not sure how carthage grabs those version numbers and I don't see a commit in LoopKit/dev that bumps the version up. can you advise? |
|
Ok - at this point I'm assuming that this is blocked on LoopKit/LoopKit#110 - once that's in master, then I can update the Cartfile to point to LoopKit 1.2.1 and we should be good to go. yes? also, unclear to me why Travis is failing - all tests run clean in my repo. |
|
The LoopKit warnings are not preventing a carthage build, right? We can merge this for people to run with Xcode 8.3, even still pointing to the LoopKit with warnings. |
|
I think so, but let me verify by going back to LoopKit 1.2.0 and see. |
|
seems to be fine. |
This reverts commit 0d486fd.
|
I take it back - that won't compile because I've already updated the various APIs to track what's in LoopKit/dev. Reverted the revert - this PR is now pegged to LoopKit/dev again. |
|
For the sake of keeping things organized, and more easily revertible, please split this into at least two PRs; one for purpose of updating Loop to Swift 3.1, which includes fixing the warnings, and recompiling frameworks, without updating those frameworks. Updates to external frameworks, and in particular, those that bring in new functionality, like the LoopKit v1.2.1 change, should be done separately. |
|
ok - so to be clear - you want:
is that right? happy to oblige. |
|
Since LoopKit 1.2.1 has functionality changes in it, and requires functional changes in Loop, that seems like it should be its own thing too. Are dexcom-share-client-swift 0.2.1, G4ShareSpy 0.3.2 just swift updates? They can maybe be postponed until we need them. |
|
The updates in G4ShareSpy and dexcom-share-client-swift are relatively minor compatibility changes - no API changes. I'll roll up a separate PR for those two - should be fine to do them together. |
|
ok, here's what we have now:
I believe I've accounted for everything - but wouldn't be surprised if I missed something in there. Will close this PR as those three supercede it. |
* LOOP-3646: Alert when CA or Notification permissions disabled * Add persistence so we only notify once * PR Feedback * Split Notifications and Critical Alerts Permissions Alerts * More splitting, added Feature Flags
3 participants
Using two forks temporarily.
Using bharat/SwiftCharts because we need the extension safe commits from
master, but don't want the rest of the master branch yet.
Using bharat/LoopKit because we want the warning fixes from
LoopKit/LoopKit#111 which is not yet merged into
LoopKit/dev