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 for watchOS 6.0.0+ #227

Merged
merged 7 commits into from
Dec 11, 2023

Conversation

scottybobandy
Copy link
Contributor

Resolves #226

This PR allows connect-swift to be compiled for watchOS.

Minimum deployment target of watchOS 6.0.0:

  • Chosen because of the use of AsyncStream, which requires watchOS 6.0.0.
  • Alternatively, an older version can be targeted, and the @available attribute used to gate AsyncStream.

As outlined in the linked issue, we cannot import CFNetwork on watchOS:

  • CFNetwork import removed from URLSessionHTTPClient.
  • fromURLSessionCode(:) was updated to pattern match on error codes explicitly, taking precedent from fromHTTPStatus(:).

Contributing checklist:

  • Test suite run and passed on all OS targets.

@CLAassistant
Copy link

CLAassistant commented Dec 7, 2023

CLA assistant check
All committers have signed the CLA.

Comment on lines +183 to +186
@available(iOS 16, *)
@available(macOS 13, *)
@available(tvOS 16, *)
@available(watchOS 9, *)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additional attributes added for minimum requirements to run the test suite on targets other than macOS.

@rebello95
Copy link
Collaborator

Thank you for submitting this! I'll review on Monday when I'm back.

Copy link
Collaborator

@rebello95 rebello95 left a comment

Choose a reason for hiding this comment

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

Thank you! I'm going to follow this up with some CI updates to build for watchOS, iOS, etc. on PRs.

@rebello95
Copy link
Collaborator

Pushed fixes to the PR to resolve CI issues (also started a Slack discussion to figure out what happened to the pinned conformance test version)

@rebello95
Copy link
Collaborator

Fixing CI here, changes ended up being a bit more involved: #228

@rebello95
Copy link
Collaborator

Pulled in CI fixes from main - my only change on this branch now is the swiftlint and docstring fix.

@rebello95 rebello95 merged commit e4f2174 into connectrpc:main Dec 11, 2023
9 checks passed
rebello95 added a commit that referenced this pull request Dec 11, 2023
- Updates CI to run tests against macOS, iOS, tvOS, and watchOS rather than only the default environment (macOS)
- Switches from using `swift test` to `xcodebuild test` since the former does not allow for specifying a target SDK/environment (see [this question](https://stackoverflow.com/questions/60245159/how-can-i-build-a-swift-package-for-ios-over-command-line) and [these notes](https://www.jessesquires.com/blog/2021/11/03/swift-package-ios-tests/))
- Updates `@available` annotations to properly accomodate these additional platforms

Follow-up to #227 and related to #226.
rebello95 added a commit that referenced this pull request Jan 9, 2024
- Adds new CI jobs to build the library against macOS, iOS, tvOS, and
watchOS rather than only the default environment (macOS)
- Updates `@available` annotations to properly accommodate these
additional platforms
- Updates CI to use Xcode 15.1

Originally the intent was to run tests on each platform, but for some
reason GitHub runners are refusing to cooperate and jobs are stalling.
Instead, we'll continue running `swift test` which runs tests on the
host environment (macOS) and will use `xcodebuild` to build for each
platform since it allows for specifying a target SDK/environment (see
[this
question](https://stackoverflow.com/questions/60245159/how-can-i-build-a-swift-package-for-ios-over-command-line)
and [these
notes](https://www.jessesquires.com/blog/2021/11/03/swift-package-ios-tests/)).

Follow-up to #227 and
related to #226.
rebello95 added a commit that referenced this pull request Jan 20, 2024
These were changed from `CFNetworkErrors` to hardcoded values in #227 to remove the `CFNetwork` import in order to support watchOS.

As an alternative, we can use `URLError` values.
rebello95 added a commit that referenced this pull request Jan 31, 2024
These were changed from `CFNetworkErrors` to hardcoded values in
#227 to remove the
`CFNetwork` import in order to support watchOS.

As an alternative, we can use `URLError` values and keep them strongly
typed. The replacements are the same except for
`CFNetworkErrors.cfurlErrorUnknown.rawValue` which will still be
correctly mapped to `.unknown` via the `...100` case.

<img width="629" alt="Screenshot 2024-01-20 at 10 07 47 AM"
src="https://github.com/connectrpc/connect-swift/assets/2643476/0d77a632-ccae-4b6a-811f-86481fddc4b7">
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.

connect-swift can't compile for watchOS
5 participants