-
-
Notifications
You must be signed in to change notification settings - Fork 871
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
refactor: Remove OCMock Carthage dependency #1754
Conversation
Thanks for opening this pull request!
|
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
Removed myself from review since there have been more commits after the request. When this is ready for review please request again from |
I would but it looks like you aren't a member of @parse-community/ios-sdk-review. Do you get notified? |
You are right; I'm now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; is this ready for merge?
Yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it seems that this is a breaking change, because it is removing watchOS 2 and 3 compatibility, right?
Package.swift is currently at .watchOS(.v2)
.
possibly breaking change, needs to be re-evaluated
Watch OS 2 and 3 are over 7 years old. Yes this could be a breaking change. |
Is the removal of watchOS 2 and 3 compatibility required to remove the OCMock Carthage dependency? |
OCMock minimum version has always been watchOS 4 For reverence Bolts-Swift is v3 |
@mtrezza If you build this PR you will see WatchOS 4 is the minimum support for Xcode 14+
Xcode won't let me choose a target less than WatchOS 4 |
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. |
@mtrezza Changes have been made. Flaky test causing CI to fail. This is ready to merge no need to restart the CI. |
Do you want to open another PR to bump the deployment target to watchOS 4? |
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. |
But why did you bump this in the first place then? |
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? |
I've mentioned my concerns about support policy of older versions in #1755. Maybe the discussion should continue there. |
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 |
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. |
🎉 This change has been released in version 3.0.0 |
* commit 'ef9b00de9eeeca7fddfe9439141f5141ff5dac02': (86 commits) chore(release): 3.0.0 [skip ci] ci: Fix release workflow (parse-community#1782) feat: Remove `ParseFacebookUtils` and `ParseTwitterUtils` (parse-community#1779) feat: Add `PFObject.isDataAvailableForKey` to check if data is available for individual key (parse-community#1756) ci: Bump ip from 2.0.0 to 2.0.1 (parse-community#1772) ci: Fix failing CI by pinning bundler version (parse-community#1778) refactor: Remove Carthage dependency (parse-community#1763) refactor: Remove OCMock Carthage dependency (parse-community#1754) ci: Run CI on Xcode 15 and macOS 13 (parse-community#1753) refactor: Update ruby dependencies (parse-community#1752) refactor: Remove framework builder for assets (parse-community#1751) chore(release): 2.7.3 [skip ci] fix: Compilation errors in `ParseLiveQuery` using `Starscream` 4.0.6 (parse-community#1749) docs: Add changelog note about missing assets (parse-community#1750) chore(release): 2.7.2 [skip ci] fix: Compilation errors `Undefined symbol` and `SystemConfiguration not found` on watchOS (parse-community#1748) ci: Remove publishing release assets (parse-community#1745) ci: Fix and build Starter Projects (parse-community#1744) chore(release): 2.7.1 [skip ci] fix: Compilation error on macOS `Undefined symbol: OBJC_CLASS$_PFProductsRequestHandler` (parse-community#1739) ...
New Pull Request Checklist
Issue Description
Closes: #1747
Approach
TODOs before merging