Skip to content
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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add support to WatchOS #286

wants to merge 8 commits into from

Conversation

linksmt
Copy link

@linksmt linksmt commented Aug 2, 2018

Checklist

  • I've checked that all new and existing tests pass
  • I've updated the documentation if necessary
  • I've added an entry in the CHANGELOG to credit myself

Description

Motivation and Context

Copy link
Owner

@AliSoftware AliSoftware left a 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 */,
Copy link
Owner

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 😉

@tfonfara
Copy link

tfonfara commented Aug 16, 2018

I'm a bit concerned if those changes are enough, as there is on several places code like #if defined(__IPHONE_7_0), that should be extended with __WATCHOS_2_0

@AliSoftware
Copy link
Owner

good point.

Copy link
Author

@linksmt linksmt left a 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

@AliSoftware
Copy link
Owner

Ping @linksmt about the #if defined(__IPHONE_7_0) remark above 😉

@tfonfara
Copy link

tfonfara commented Dec 5, 2018

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.
canInitWithRequest is being called and also returns true, but neither initWithRequest nor startLoading gets called. I also tried with a simple approach in a fresh project to implement NSURLProtocol but found same behaviour.

@linksmt
Copy link
Author

linksmt commented Mar 30, 2019

Can you accept merge?
Thanks

@jeffctown
Copy link
Collaborator

@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 CHANGELOG.md to give yourself credit and update the original description of this PR.

@1ec5

This comment has been minimized.

@Juanpe
Copy link

Juanpe commented Aug 6, 2021

Hi! @AliSoftware @linksmt, what is the status of this PR? I mean, OHHTTPStubs is still not supporting watchOS, doesn't it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants