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

Adds support for Carthage #173

Merged
merged 5 commits into from
Sep 10, 2017
Merged

Adds support for Carthage #173

merged 5 commits into from
Sep 10, 2017

Conversation

elitalon
Copy link
Contributor

@elitalon elitalon commented Aug 4, 2017

This attempts to fix #74 by solving a few build errors that happen when building with carthage --no-skip-current.

Theoretically, that should allow projects managed with Carthage to use the framework.

@brototyp
Copy link
Member

brototyp commented Aug 5, 2017

Hi @elitalon, thank you very much for your effort! I am not used to use Carthage. Is there any way we can add a Carthage-Build check to the travis file?

@elitalon
Copy link
Contributor Author

elitalon commented Aug 8, 2017

@brototyp I'll see what I can do. I think running carthage build --no-skip-current would be a first step, since it's what they recommend as a first diagnostic.

@elitalon
Copy link
Contributor Author

elitalon commented Sep 1, 2017

@brototyp There's a problem with TrackerSpec.swift:70when running tests on iOS 9.3:

let _ = Timer.scheduledTimer(withTimeInterval: 0.1, repeats: true) { timer in
    trackerFixture.tracker.queue(event: EventFixture.event())
}

Basically, scheduledTimer(withTimeInterval:repeats:block:) is only available in iOS 10+. Do you have any suggestions on how to fix this?

A quick dirty solution would be using GCD:

if #available(iOS 10, *) {
    let _ = Timer.scheduledTimer(withTimeInterval: 0.1, repeats: true) { timer in
        trackerFixture.tracker.queue(event: EventFixture.event())
    }
} else {
    var scheduleEventTracking: (() -> Void)?
    scheduleEventTracking = {
        guard numberOfDispatches < 5 else {
            return
        }

        DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
            trackerFixture.tracker.queue(event: EventFixture.event())
            scheduleEventTracking?()
        }
    }
    scheduleEventTracking?()
}

What do you think?

@brototyp
Copy link
Member

brototyp commented Sep 4, 2017

@elitalon Your solution might result in wrong test fails. If the trackerFixture.tracker.queue happens two times in between dispatches there will only be 4 dispatches in total and the test fails. I will fix this issue in a separate test branch.

@brototyp
Copy link
Member

brototyp commented Sep 4, 2017

@elitalon What do you think about #179?

@elitalon
Copy link
Contributor Author

elitalon commented Sep 4, 2017

@brototyp I already commented on #179, much better and without branching on iOS versions. As soon as it gets merged I'll rebase here.

@brototyp
Copy link
Member

brototyp commented Sep 4, 2017

Great, thanks! #179 is merged.

@elitalon elitalon closed this Sep 4, 2017
@elitalon elitalon deleted the feature/add-carthage-support branch September 4, 2017 19:12
@elitalon elitalon restored the feature/add-carthage-support branch September 4, 2017 19:32
@elitalon elitalon reopened this Sep 4, 2017
Running `pod install` with CocoaPods 1.3.0 caused some project settings
to be updated.
For some reason `carthage build --no-skip-current` (and later xcodebuild)
complaint about not being able to import UIKit in PiwikTracker.h.
`tvos` has been replaced by `appletvos`.
@elitalon
Copy link
Contributor Author

elitalon commented Sep 7, 2017

@brototyp The PR is again ready, but something happened with the Travis build. Do you have any idea? I don't have rights to trigger it again.

@brototyp
Copy link
Member

@elitalon I just started the build once again (a third time) and it all went through. Thanks!

@brototyp brototyp merged commit f88b24f into matomo-org:develop Sep 10, 2017
@elitalon elitalon deleted the feature/add-carthage-support branch September 11, 2017 08:45
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.

Add support for dependency management using Carthage
2 participants