search

zenangst / Family

:children_crossing: A child view controller framework that makes setting up your parent controllers as easy as pie.
Other
250 stars 10 forks source link

Add Swift Package Manager support #187

Closed zocario closed 3 years ago

zocario commented 4 years ago

Hello I'm trying to give a little help regarding support of Swift Package Manager as I need it in one of my project :) This is a very simple implementation I just did and allowed me to use Family with SPM recently. I'm not sure if this is good enough but I think it is a good base to start with if it helps.

Changes

zenangst commented 4 years ago

@zocario Good start mate and thanks for starting to add support for this. Seems like there is still some additional work to be done on the macOS target, mind taking a look at that?

zocario commented 4 years ago

@zenangst Not today but I can have a look for macOS at the end of week :)

zenangst commented 4 years ago

No worries, I just check to see why Travis wasn't playing ball. Cheers!

zocario commented 4 years ago

I also forgot to add tests targets, I'll polish this and mention you again ;)

zocario commented 4 years ago

@zenangst I had a second look and fixed macOS compilation when opening swift package. But I'm facing two failures test on iOS+tvOS:

I'm not really sure to see how this has changed with my modifications to be honest. If you have time to give your opinion on the changes I've started don't hesitate :)

zocario commented 4 years ago

Also to me we shouldn't need to use the xcodeproj anymore, but I'm not sure we can work without it regarding travis CI. I'll need to update the xcodeproj to work with the changes I made I'll update this soon.

codecov-commenter commented 4 years ago

Codecov Report

Merging #187 into master will decrease coverage by 1.29%. The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #187      +/-   ##
==========================================
- Coverage   81.45%   80.16%   -1.30%     
==========================================
  Files          11       11              
  Lines         836      837       +1     
==========================================
- Hits          681      671      -10     
- Misses        155      166      +11
Flag Coverage Δ
#iOS 74.25% <90.00%> (-9.75%) :arrow_down:
#macOS 80.16% <90.47%> (+0.02%) :arrow_up:
#tvOS 74.25% <90.00%> (+31.25%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Sources/macOS/Classes/FamilyDocumentView.swift 91.07% <ø> (ø)
Sources/macOS/Classes/FamilyScrollView.swift 81.52% <ø> (ø)
Sources/macOS/Classes/FamilyWrapperView.swift 83.33% <ø> (ø)
...ces/macOS/Extensions/NSScrollView+Extensions.swift 100.00% <ø> (ø)
...macOS/Extensions/NSViewController+Extensions.swift 66.66% <ø> (ø)
Sources/Shared/FamilySpaceManager.swift 56.25% <83.33%> (-27.63%) :arrow_down:
Sources/Shared/FamilyCache.swift 100.00% <100.00%> (ø)
...ources/Shared/FamilyViewControllerAttributes.swift 84.21% <100.00%> (ø)
Sources/macOS/Classes/FamilyViewController.swift 78.81% <100.00%> (ø)
Sources/Shared/BinarySearch.swift 73.52% <0.00%> (-5.89%) :arrow_down:

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 c9105ff...3c10ce8. Read the comment docs.

zocario commented 4 years ago

@zenangst So I succeed to setup Travis by using the demo project as container of the swift package. It is using the local swift package and exposes its schemes to be run by Travis. I don't know if this setup would be fine for you (I think it should work for Carthage and Cocoapods but needs to confirm) but this is how I'm doing currently at work to get CI running with swift packages. The constraint is just that the xcodeproj can't be at the root level with this configuration that's why I've used the demo project.

zenangst commented 4 years ago

Looking good, I was actually thinking about removing the project all together and perhaps use XcodeGen instead so I think that change would be fine.

zenangst commented 4 years ago

Don't have time to do a full review now but perhaps early next week if again… life allows 😎

zenangst commented 4 years ago

@zocario mind fixing up the merge conflicts here? (totally my bad 😶)

zocario commented 4 years ago

@zenangst Done :)

zocario commented 4 years ago

Also I have a test failure on Mac OS but nothing fails locally for me (10.15.7 Travis is 10.15.6), would you mind trying on your side?

codecov-io commented 4 years ago

Codecov Report

Merging #187 into master will decrease coverage by 1.29%. The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #187      +/-   ##
==========================================
- Coverage   81.45%   80.16%   -1.30%     
==========================================
  Files          11       11              
  Lines         836      837       +1     
==========================================
- Hits          681      671      -10     
- Misses        155      166      +11
Flag Coverage Δ
#iOS 74.25% <90.00%> (-9.75%) :arrow_down:
#macOS 80.16% <90.47%> (+0.02%) :arrow_up:
#tvOS 74.25% <90.00%> (+31.25%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Sources/macOS/Classes/FamilyDocumentView.swift 91.07% <ø> (ø)
Sources/macOS/Classes/FamilyScrollView.swift 81.52% <ø> (ø)
Sources/macOS/Classes/FamilyWrapperView.swift 83.33% <ø> (ø)
...ces/macOS/Extensions/NSScrollView+Extensions.swift 100.00% <ø> (ø)
...macOS/Extensions/NSViewController+Extensions.swift 66.66% <ø> (ø)
Sources/Shared/FamilySpaceManager.swift 56.25% <83.33%> (-27.63%) :arrow_down:
Sources/Shared/FamilyCache.swift 100.00% <100.00%> (ø)
...ources/Shared/FamilyViewControllerAttributes.swift 84.21% <100.00%> (ø)
Sources/macOS/Classes/FamilyViewController.swift 78.81% <100.00%> (ø)
Sources/Shared/BinarySearch.swift 73.52% <0.00%> (-5.89%) :arrow_down:

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 c9105ff...7669cf6. Read the comment docs.

zenangst commented 4 years ago

Hey @zocario, I tried this branch and I was unable to install it using CocoaPods. I ended up with the following error inside of FamilyScrollView

image
zenangst commented 4 years ago

I'm curious if we could refactor the Package.swift to only produce one framework per platform. I think that would fix the issue with the module issue that arises when trying to use it with CocoaPods.

zocario commented 3 years ago

Hey @zenangst I'm sorry I completely missed your last message, I'll take an other look this week-end, big weeks at work at the moment 🙈

zenangst commented 3 years ago

@zocario no worries mate :)

zenangst commented 3 years ago

@zocario Hmm, I'm still having issues installing this using CocoaPods

zenangst commented 3 years ago

@zocario I opened a new PR here - https://github.com/zenangst/Family/pull/207 that takes a different approach. Instead of creating a shared framework, it uses pre-conditions per platform. The major benefit here is that it still works when using it with CocoaPods.

zocario commented 3 years ago

@zenangst Looks great! Way better approach that my try :) To be honest I wasn't sure about how to do this in an other way, and I had a lots of things to do at work. I should have thought about pre-conditions! I close the PR ;)

zenangst commented 3 years ago

@zocario no worries mate, thanks for all the hard work that you put into this! ❤️