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

Added watchOS support #91

Merged
merged 5 commits into from Oct 21, 2019
Merged

Added watchOS support #91

merged 5 commits into from Oct 21, 2019

Conversation

jklp
Copy link
Contributor

@jklp jklp commented May 13, 2019

Added a target for watchOS. Couldn't add the test target as couldn't get Nimble/Quick to build on watchOS.

@jklp jklp requested a review from a team May 13, 2019 04:23
@jklp jklp mentioned this pull request May 13, 2019
@damieng damieng added the medium label May 22, 2019
@cocojoe
Copy link
Member

cocojoe commented May 23, 2019

Apologies I've not had time to try this, it looks okay at a glance, although not being able to run test is not ideal. I would like to take a look at that, to set expectation I'm on vacation so will be a bit delayed in response. Thanks

@jklp
Copy link
Contributor Author

jklp commented May 24, 2019

No worries, fwiw we've been running on WatchOS for the last couple weeks here and haven't run into any problems 🙂

@cocojoe
Copy link
Member

cocojoe commented Jun 17, 2019

@jklp please update README with watchOS support.

@cocojoe cocojoe requested review from cocojoe and removed request for a team June 17, 2019 08:52
Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

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

In general looks fine, just update README and PodSpec, check dependency version against project.

@cocojoe cocojoe self-assigned this Jun 17, 2019
@jklp
Copy link
Contributor Author

jklp commented Jun 19, 2019

@cocojoe Hi, I updated the podspec and tested (and it works) though I might need some help with the other 2 items. I'm not 100% on what to update in the README (I can't find anything which would be platform specific to watchOS) and I'm not sure what you mean by the dependency version (sorry!)

@cocojoe
Copy link
Member

cocojoe commented Jun 19, 2019

@jklp fair question as there was no existing WatchOS support, what's the minimum version that can be specified in the project? Would be ideal to match our other libraries e.g. https://github.com/auth0/Auth0.swift#requirements

When I said dependency I mean the deployment version in the PodSpec should match the project.

I would use the newer swift versions syntax as well which is demonstrated here for language version support in this PR. auth0/Auth0.swift#279

@thedavidharris thedavidharris mentioned this pull request Jul 12, 2019
1 task
@jklp
Copy link
Contributor Author

jklp commented Aug 15, 2019

@cocojoe Hey sorry for the long delay. I've just updated the README.md and also dropped the watchOS version to 2.0 to match the rest the project. Please let me know if there's anything else. Thanks!

@dylanmoo
Copy link

Is this pull request ready to be merged? With Auth0 SIWA available on apple watch as of iOS 13, it would be helpful to be able to read the JWT token on the Watch Extension itself.

Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

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

I tried it out, looks okay 👍

@cocojoe cocojoe requested a review from a team October 20, 2019 15:55
@cocojoe cocojoe changed the title Adding watchOS target Added watchOS target Oct 20, 2019
@cocojoe cocojoe changed the title Added watchOS target Added watchOS support Oct 20, 2019
@cocojoe
Copy link
Member

cocojoe commented Oct 20, 2019

#58

@cocojoe cocojoe added this to the vNext milestone Oct 21, 2019
@cocojoe cocojoe merged commit 8db1598 into auth0:master Oct 21, 2019
@Widcket Widcket mentioned this pull request Nov 27, 2019
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

4 participants