parse-community / Parse-SDK-iOS-OSX

The Apple SDK for Parse Platform (iOS, macOS, watchOS, tvOS)
https://parseplatform.org
Other
2.81k stars 865 forks source link

refactor: Remove OCMock Carthage dependency #1754

Closed dplewis closed 9 months ago

dplewis commented 9 months ago

New Pull Request Checklist

Issue Description

Closes: https://github.com/parse-community/Parse-SDK-iOS-OSX/issues/1747

Approach

TODOs before merging

parse-github-assistant[bot] commented 9 months ago

Thanks for opening this pull request!

codecov[bot] commented 9 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (c4998e6) 11.24% compared to head (137e9a5) 64.40%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1754 +/- ## =========================================== + Coverage 11.24% 64.40% +53.16% =========================================== Files 195 200 +5 Lines 23015 23176 +161 =========================================== + Hits 2587 14927 +12340 + Misses 20428 8249 -12179 ``` [see 141 files with indirect coverage changes](https://app.codecov.io/gh/parse-community/Parse-SDK-iOS-OSX/pull/1754/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mtrezza commented 9 months ago

Removed myself from review since there have been more commits after the request. When this is ready for review please request again from @parse-community/ios-sdk-review

dplewis commented 9 months ago

I would but it looks like you aren't a member of @parse-community/ios-sdk-review. Do you get notified?

mtrezza commented 9 months ago

You are right; I'm now

dplewis commented 9 months ago

Yes

dplewis commented 9 months ago

Watch OS 2 and 3 are over 7 years old. Yes this could be a breaking change.

mtrezza commented 9 months ago

Is the removal of watchOS 2 and 3 compatibility required to remove the OCMock Carthage dependency?

dplewis commented 9 months ago

OCMock minimum version has always been watchOS 4

For reverence

Bolts-Swift is v3 Bolts-Objc is v2 Starscream is v2

dplewis commented 9 months ago

@mtrezza If you build this PR you will see dependency 'OCMock' is not used by any target. It's basically a dev dependency.

WatchOS 4 is the minimum support for Xcode 14+

The Xcode 14 release supports on-device debugging in iOS 11 and later, tvOS 11 and later, and watchOS 4 and later. Xcode 14 requires a Mac running macOS Monterey 12.5 or later.

Xcode won't let me choose a target less than WatchOS 4

Screenshot 2023-10-06 at 8 29 59 PM
mtrezza commented 9 months ago

So the watchOS bump is unrelated to the removal of OCMock? If that is so, I think it would be better to do that in a separate PR.

dplewis commented 9 months ago

@mtrezza Changes have been made. Flaky test causing CI to fail. This is ready to merge no need to restart the CI.

mtrezza commented 9 months ago

Do you want to open another PR to bump the deployment target to watchOS 4?

dplewis commented 9 months ago

We support Xcode 14+, any developer making a WatchOS app is forced to use WatchOS 4+. 4 > 2 so there will never be an issue. If the Repo was developed specifically for WatchOS then I would do a PR.

mtrezza commented 9 months ago

But why did you bump this in the first place then?

dplewis commented 9 months ago

I thought I was right at the time. I'm still learning. This project hasn't been worked on in a long time. Maybe I'm still right. I know why this project supports iOS 12 as a minimum do you?

mtrezza commented 9 months ago

I've mentioned my concerns about support policy of older versions in https://github.com/parse-community/Parse-SDK-iOS-OSX/issues/1755. Maybe the discussion should continue there.

dplewis commented 9 months ago

Do what you have to, just don't increase the CI build time. Thats the only reason by I'm working on this project right now. I don't work on the parse-server anymore because of the / your policy

mtrezza commented 9 months ago

Not sure what you mean by that, but as always please feel free to open an issue in the Parse Server repository if you feel that a policy there is making it difficult for you to contribute.

parseplatformorg commented 3 months ago

🎉 This change has been released in version 3.0.0