Skip to content
This repository was archived by the owner on Jan 5, 2024. It is now read-only.

Add animated parameter#30

Merged
gigisommo merged 4 commits into
justeat:masterfrom
gigisommo:feature/add-animated-parameter
Jul 8, 2019
Merged

Add animated parameter#30
gigisommo merged 4 commits into
justeat:masterfrom
gigisommo:feature/add-animated-parameter

Conversation

@gigisommo

Copy link
Copy Markdown
Contributor

This PR adds the possibility to pass an animated parameter to present and remove view controllers with or without animation.

I moved the supported operating system to iOS 11.

This PR bumps the major version (to 5.0.0)

@richardtop

Copy link
Copy Markdown

Hi, thanks for the pull request, however, for me it's important to support iOS 9+. Could you please apply the changes without bumping the deployment target?

What are the new APIs that are critical for this feature to work?

@gigisommo

gigisommo commented Jul 5, 2019

Copy link
Copy Markdown
Contributor Author

I'm sorry for that, but we chose to move all our deployment target to iOS 11. As far as I know there aren't APIs that prevent it to work on iOS 9, but we're trying to follow Apple's advice on supporting just the iOS major version before the current major one. I would advise you to fork it(and change the deployment target) because we're not going to work on compatibility for versions previous to iOS 11 not just in terms of API, but also considering testing and eventual bug fixes.

@richardtop

Copy link
Copy Markdown

@gigisommo Could you then not bump it in this update? For example, mention that the officially supported version is 11+, but keep the one specified in the .podspec as 9+.

That would make it able to install for me, without the need to fork and do the extra maintenance.

@richardtop

Copy link
Copy Markdown

Alternatively, bump the version to iOS 11 after this pull request gets merged, during the next release of the library.

We both are happy:

  1. You keep iOS 11+ as the minimum deployment target
  2. I incorporate the change I need in my project without forking it.

@gigisommo

Copy link
Copy Markdown
Contributor Author

Yes, it make sense. I'll open another PR to remove the support to iOS 9 after this one. I'm reverting the deployment target changes

@richardtop

Copy link
Copy Markdown

Thanks a lot!

@gigisommo

Copy link
Copy Markdown
Contributor Author

You're welcome. Consider pinning the version to the 5.0.0 because we're going to change the deployment target soon.

@richardtop

Copy link
Copy Markdown

Fine. We're too :) It's just now I have to support iOS 9.

@gigisommo gigisommo merged commit 2fc8bc1 into justeat:master Jul 8, 2019
@gigisommo gigisommo deleted the feature/add-animated-parameter branch July 8, 2019 09:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants