Skip to content

Lower deployment target and add availability checks to all Combine related code#29

Merged
freak4pc merged 1 commit intoCombineCommunity:masterfrom
RonKliffer:master
May 10, 2020
Merged

Lower deployment target and add availability checks to all Combine related code#29
freak4pc merged 1 commit intoCombineCommunity:masterfrom
RonKliffer:master

Conversation

@RonKliffer
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented May 3, 2020

Codecov Report

Merging #29 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #29   +/-   ##
=======================================
  Coverage   97.41%   97.41%           
=======================================
  Files          38       38           
  Lines        2244     2244           
=======================================
  Hits         2186     2186           
  Misses         58       58           
Impacted Files Coverage Δ
Sources/Common/DemandBuffer.swift 100.00% <ø> (ø)
Sources/Common/Sink.swift 57.50% <ø> (ø)
Sources/Models/Event.swift 52.63% <ø> (ø)
Sources/Operators/Amb.swift 100.00% <ø> (ø)
Sources/Operators/AssignToMany.swift 100.00% <ø> (ø)
Sources/Operators/CombineLatestMany.swift 100.00% <ø> (ø)
Sources/Operators/Create.swift 100.00% <ø> (ø)
Sources/Operators/Dematerialize.swift 85.18% <ø> (ø)
Sources/Operators/FlatMapLatest.swift 100.00% <ø> (ø)
Sources/Operators/MapMany.swift 100.00% <ø> (ø)
... and 28 more

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 5bd0d0f...f1cec3b. Read the comment docs.

@RonKliffer RonKliffer changed the title Lower podspec iOS deployment target to 11.0 and add available(iOS 130) to all Combine related code Lower podspec iOS deployment target to 11.0 and add available(iOS 13.0) to all Combine related code May 3, 2020
@freak4pc
Copy link
Member

freak4pc commented May 4, 2020

Me and @RonKliffer had a discussion around this yesterday, after I've heard this use-case from several people.

There are people who actually use Combine in newer portions of their apps, e.g. iOS 11 apps that want to use this dependency with an availability clause. I think it's a valid use-case overall. WDYT @jasdev @jdisho ?


import Combine

@available(iOS 13.0, *)
Copy link
Member

Choose a reason for hiding this comment

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

All tests should stay iOS 13 and up. Only the target itself should be iOS 11+
I'm actually not sure it's possible, but worth checking

@jdisho
Copy link
Contributor

jdisho commented May 4, 2020

Seems valid case to me too.👌

@jdisho
Copy link
Contributor

jdisho commented May 4, 2020

Should we do the same thing for SPM?

@jasdev
Copy link
Member

jasdev commented May 4, 2020

WDYT @jasdev @jdisho?

Cool with me—no concerns on my end as long as you also think this is the right call for 11.x+ folks. 👌🏽

@RonKliffer RonKliffer changed the title Lower podspec iOS deployment target to 11.0 and add available(iOS 13.0) to all Combine related code Lower deployment target and add availability checks to all Combine related code May 10, 2020
Copy link
Member

@freak4pc freak4pc left a comment

Choose a reason for hiding this comment

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

Thank you @RonKliffer ! Ran everything locally and this looks good 💯

@freak4pc freak4pc merged commit 357e62b into CombineCommunity:master May 10, 2020
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