-
Notifications
You must be signed in to change notification settings - Fork 601
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
Add support to WatchOS #286
base: master
Are you sure you want to change the base?
Conversation
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.
Addition of watchOS support would be cool indeed, thanks for the PR 👍
tbh, I'm surprised it would be compatible at all (like, is it possible now to emit requests via URLSession & all… directly from watchOS, not needing WatchConnectivity to ask the phone to do it for the watch? And thus for OHHTTPStubs to intercept them? I guess so, now that they introduced the latest AppleWatches… But I haven't done any watchOS development in a while and didn't follow that topic much, so probably a lot have changed that I didn't follow 😄
Waiting for you to finish the PR (CHANGELOG, tests, example project to show it at work…) to validate it 😉
@@ -374,6 +403,7 @@ | |||
09110A4319805F4800D175E4 /* Frameworks */, | |||
09110A4219805F4800D175E4 /* Products */, | |||
71E7CDE1C6A8345F6C70E7D1 /* Pods */, | |||
C8EDD4532111DF8200E60E4E /* OHHTTPStubs iOS Framework copy-Info.plist */, |
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.
Please rename that file appropriately for consistency 😉
I'm a bit concerned if those changes are enough, as there is on several places code like |
good point. |
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.
Change name of watchos plist as suggested
Ping @linksmt about the |
I tried to come up with a solution by myself, but things are petty weird. I've added all necessary precompiler defines, but OHHTTPStubsProtocol isn't working properly. |
Can you accept merge? |
@linksmt thank you for submitting this! Everything looks great for the added target, but I think we should probably have some tests to go along with it so we can make sure we don't break it. 😄 Would you be comfortable adding some tests to the watchOS target? I think it would also be helpful if you update the |
This comment has been minimized.
This comment has been minimized.
Hi! @AliSoftware @linksmt, what is the status of this PR? I mean, |
Checklist
Description
Motivation and Context