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

Disable ThrowAssertion for watchOS to make Nimble compile for watchOS #953

Merged
merged 2 commits into from
Apr 15, 2022

Conversation

danielsaidi
Copy link
Contributor

@danielsaidi danielsaidi commented Dec 30, 2021

The PR should summarize what was changed and why. Here are some questions to
help you if you're not sure:

  • What behavior was changed?
  • What code was refactored / updated to support this change?
  • What issues are related to this PR? Or why was this change introduced?

Checklist - While not every PR needs it, new features should consider this list:

  • Does this have tests?
  • Does this have documentation?
  • Does this break the public API (Requires major version bump)?
  • Is this a new feature (Requires minor version bump)?

@danielsaidi
Copy link
Contributor Author

danielsaidi commented Dec 30, 2021

I have had problem with Nimble for a long time, when developing multi-platform libraries for watchOS or watchOS apps. Nimble just won't compile, since the BadInstructionException isn't defined for watchOS.

I made Nimble compile on watchOS by adding an #if switch to throwAssertion, that throws a fatal error for watchOS, much like it already does for other unsupported platforms.

This means that throwAssertion will always fail for watchOS, but at least the code will compile and any developers that use throwAssertion will get a descriptive error and have the choice to disable these tests for watchOS.

I also disabled ThrowAssertionTests for watchOS, and can confirm that the Nimble unit tests run perfectly on watchOS.

There is already another PR for watchOS, but this is much smaller and should be easier to merge. Perhaps we can merge this as a first step, before taking on the other PR that has been waiting since June?

If we merge this PR, then create a new patch version for Nimble and use that in Quick, both libraries will hopefully compile for and support watchOS, with the only small caveat that throwAssertion currently doesn't work as expected on watchOS.

@danielsaidi danielsaidi changed the title Disable ThrowAssertion for watchOS Disable ThrowAssertion for watchOS to make Nimble compile for watchOS Dec 30, 2021
@younata younata added this to the v10 milestone Apr 14, 2022
@younata
Copy link
Member

younata commented Apr 15, 2022

Hey @danielsaidi, this makes sense. I updated your branch with latest main.

@younata younata merged commit fc9c27f into Quick:main Apr 15, 2022
@younata younata mentioned this pull request Apr 15, 2022
@danielsaidi
Copy link
Contributor Author

Hey @younata - thank you! I'm looking forward to a new release with this fix :)

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

Successfully merging this pull request may close these issues.

2 participants