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

Reducing deployment target #17

Merged
merged 4 commits into from
Nov 25, 2016
Merged

Reducing deployment target #17

merged 4 commits into from
Nov 25, 2016

Conversation

tbaranes
Copy link
Collaborator

@tbaranes tbaranes commented Nov 24, 2016

That framework looks really great, but the deployment target is quiet hight for my projects 😬

So, I updated (for carthage and CocoaPods) them to:

  • iOS: 8.0
  • macOS: 10.0
  • tvOS: 9.0 (no need for that one, but I think it's for the best)

Is there any reasons to have higher version? I didn't see any regressions by running the tests / launching the example.

@wokalski
Copy link
Owner

Thanks! We actually need to make CI work on those versions, too. In order to accomplish that we need to tweak Travis setup. It is currently being changed in #16 so I'd wait until it's merged.

Thanks again. I'll notify you when the other PR is merged.

@wokalski
Copy link
Owner

@tbaranes OK, #16 is merged. Feel free to rebase (or really, it will be easier to just reapply the changes 😜) and update .travis.yml (add entries to the matrix)

@tbaranes
Copy link
Collaborator Author

tbaranes commented Nov 25, 2016

Just rebased, I see that the other PR has already reduced the xcodeproj targets, so it will just for CocoaPods!

Travis up to date too, I added all the os destinations supported by Diff.swift. Let met know if there's something missing

@tbaranes
Copy link
Collaborator Author

Any ideas why the iOS10+ targets are failing? 🤔

@wokalski
Copy link
Owner

If you view the raw log on Travis you can see that it failed because of a timeout. I saw some discussions about it online but can't find them anymore. @tbaranes.

If you can't find them I'll do my best to fix it over the weekend

@tonyarnold
Copy link
Collaborator

Yeah, you're looking for this: travis-ci/travis-ci#4725

At the bottom of .travis.yml, update it to:

after_success:
   - bash <(curl -s https://codecov.io/bash)
   - sleep 5 # Workaround for https://github.com/travis-ci/travis-ci/issues/4725

@tbaranes
Copy link
Collaborator Author

tbaranes commented Nov 25, 2016

Right, I already see that kind of issue on others projects.
I updated .travis using @tonyarnold answer, thanks guys!

after_success:
- bash <(curl -s https://codecov.io/bash)
- sleep 5 # Workaround for https://github.com/travis-ci/travis-ci/issues/4725
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small note - please ensure there are newlines at the end of all of your files.

Otherwise, all 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated 👌

@codecov-io
Copy link

codecov-io commented Nov 25, 2016

Current coverage is 93.23% (diff: 100%)

No coverage report found for master at bfac415.

Powered by Codecov. Last update bfac415...525228f

@wokalski wokalski merged commit 7b621dc into wokalski:master Nov 25, 2016
@wokalski
Copy link
Owner

Awesome @tbaranes. Thanks a lot!

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.

4 participants