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

Swift 4 #23

Merged
merged 36 commits into from Sep 26, 2017
Merged

Swift 4 #23

merged 36 commits into from Sep 26, 2017

Conversation

ksmandersen
Copy link
Contributor

  • Added full support for Swift 4.0
  • Fixed a bunch of warnings
  • Updated to recommended project settings

elm4ward and others added 29 commits May 5, 2016 07:46
PalauDefaultable got 2 new methods:
get(key: String, from defaults: NSUD) -> [ValueType]?
and
set(value: ValueType?, forKey key: String, in defaults: NSUD) -> Void

In the current version the PalauDefaultsArrayEntry is more or less a
copy of PalauDefaultsEntry. There should be good chance that we can
abstract some things. Also copied the tests and converted them to use
PalauDefaultsArrayEntry.
PalauEntry Protocol is used as base for PalauDefaultsEntry and
PalauDefaultsArrayEntry. Most of the methods can be defined in an
extension of PalauEntry.

PalauCustomDefaultable is used to provide easy support for e.g. Struct
mapping either for PalauDefaultsEntry or PalauDefaultsArrayEntry.
The implementation of PalauCustomDefaultable needs to provide a
toIntermediate and fromIntermediate func.
Added swiftlint to all target builds so its never missed
Removed support for NSURL and NSIndexPath for the time being.
The PalauTestCase holds a new shiny cleanUpSimulatorDefaultsForSure()
method which (at least on my machine) made tests predictable again.
Further more checkValue is rewritten in cleaner Swift 3 syntax.
@madhavajay
Copy link
Contributor

Let me take a quick look. 👍

@ksmandersen
Copy link
Contributor Author

@madhavajay Let's wait until the travis build completes. Had to tweak some things, because swiftlint makes it fail

@elm4ward
Copy link
Member

Wow - @ksmandersen Thank you very much.
I was planning to make an update and basically boil it down to Codable.
But your PR is already very welcome!

👍🙏🎊

@ksmandersen
Copy link
Contributor Author

Now swiftlint works on travis but the tests fail. Not sure what is up with that. They all pass locally. Anyone got some insight?

@madhavajay
Copy link
Contributor

@ksmandersen its going to be the simulator versions, its a total pain every time they change. I would actually suggest changing this to use fastlane.

First would be to create a fastfile using fastlane, then to invoke that in the travis file.

@ksmandersen
Copy link
Contributor Author

@madhavajay That seems like a separate concern from simply supporting Swift4/Xcode9. I also haven't used fastlane before, so am not familiar with how it works.

I've updated the simulator versions. So let's see if that doesn't work for now

@madhavajay
Copy link
Contributor

@ksmandersen yeah if updating the simulators works thats fine for now. 👍

For your own sake, please go checkout fastlane like right now.
It will blow your mind and rock your iOS world.
Seriously, its like.... the missing toolchain for iOS dev. 😎

@ksmandersen
Copy link
Contributor Author

Looks like the tests will pass now

@madhavajay
Copy link
Contributor

I think its just the watch device build failing now due to incorrect device name. Really annoying that Xcode does this.

Also can you please bump the version to say: 1.0.4 in the plist files?

madhavajay and others added 3 commits September 26, 2017 15:50
- Updated .travis.yml file to use fastlane
- Bumped version to 1.0.4 in plists and podspec
@ksmandersen
Copy link
Contributor Author

@madhavajay Let's try that again.

Bumped to version 1.0.4 now

@madhavajay
Copy link
Contributor

@ksmandersen I just made a PR to your fork from my fork with the new fastlane and travis changes. Can you merge that and then it should get added to this PR.

@ksmandersen
Copy link
Contributor Author

@madhavajay Done

@elm4ward
Copy link
Member

cool

@madhavajay
Copy link
Contributor

Woohoo. 🎉 Good job everyone! 👍

@codecov-io
Copy link

Codecov Report

Merging #23 into master will increase coverage by <.01%.
The diff coverage is 94.27%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #23      +/-   ##
=========================================
+ Coverage    93.8%   93.8%   +<.01%     
=========================================
  Files           4       3       -1     
  Lines         371     581     +210     
=========================================
+ Hits          348     545     +197     
- Misses         23      36      +13
Impacted Files Coverage Δ
Tests/PalauTestHelpers.swift 100% <100%> (ø)
Tests/PalauDefaultsArrayTest.swift 93.45% <93.45%> (ø)
Tests/PalauTests.swift 93.68% <95.45%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3af20e...860ab84. Read the comment docs.

@madhavajay madhavajay merged commit f4b50ef into symentis:master Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants