Skip to content

swift: add public methods for published states and update docs #37

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

Merged
merged 1 commit into from
May 5, 2025

Conversation

barnstar
Copy link
Member

@barnstar barnstar commented May 5, 2025

updates tailscale/tailscale#15802

The @published connection states weren't public. These are now exposed as AsyncSequences
so that there's no public dependency on Combine.

Documentation cleaned up and clarified slightly. Some minor reorganization of localAPI.

Adds a test to ensure that localAPI is functional.

Add the xcode equivalent of -wall -werror

@barnstar barnstar requested a review from Copilot May 5, 2025 18:01
Copy link

review-ai-agent bot commented May 5, 2025

Pull Request Revisions

RevisionDescription
r4
Refactored Tailscale Swift API implementationMade several code improvements including error handling, method signatures, state management, and renamed some configuration variables across multiple Swift files
r3
Fixed typos and improved code documentationMade minor documentation improvements, fixed typos, added doc comments, and made some transformer methods private
r2
Improved error handling in testsAdded descriptive error messages to XCTFail() calls in test cases to provide more context when tests fail
r1
Improved Swift codebase with warningsAdded numerous Clang and Swift compiler warnings, adjusted code structure, and enhanced build configurations for TailscaleKit

✅ AI review completed for r4
Help React with emojis to give feedback on AI-generated reviews:
  • 👍 means the feedback was helpful and actionable
  • 👎 means the feedback was incorrect or unhelpful
💬 Replying to feedback with a comment helps us improve the system. Your input also contributes to shaping future interactions with the AI reviewer.

We'd love to hear from you—reach out anytime at [email protected].

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR exposes previously internal published states as async streams, updates documentation, and adds tests to verify localAPI functionality. Key changes include:

  • Updating TailscaleKitTests to safely convert C-style strings to Swift Strings.
  • Renaming and exposing connection/listener states via async sequences.
  • Refining LocalAPIClient method parameters and documentation.

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
swift/TailscaleKitXCTests/TailscaleKitTests.swift Enhanced string conversion for controlURL and added tests for localAPI status.
swift/TailscaleKit/OutgoingConnection.swift Corrected a typo in the enum name from "ListenterState" to "ListenerState".
swift/TailscaleKit/LocalAPI/LocalAPIClient.swift Updated parameter naming in editPrefs and improved doc comments.
swift/TailscaleKit/Listener.swift Refactored the published state to _state and added async state stream access.
swift/TailscaleKit/IncomingConnection.swift Refactored published state handling to use async state streams.
Files not reviewed (2)
  • swift/Makefile: Language not supported
  • swift/TailscaleKit.xcodeproj/project.pbxproj: Language not supported

@barnstar barnstar force-pushed the barnstar/swift-cleanup branch from 8f9fe5d to 31435e6 Compare May 5, 2025 18:07
@barnstar barnstar force-pushed the barnstar/swift-cleanup branch from 31435e6 to 1e5f359 Compare May 5, 2025 18:14
updates tailscale/tailscale#15802

The connection states weren't public.  These are now exposed as
AsyncSequences so that there's no public dependency on Combine.

Documentation cleanup up and clarified slightly.  Some minor
reorganization of localAPI.   Adds a test to ensure that localAPI
is functional.

Add the xcode equivalent of -wall -werror
@barnstar barnstar force-pushed the barnstar/swift-cleanup branch from 1e5f359 to 4adbbcc Compare May 5, 2025 18:41
@barnstar barnstar merged commit 78294ac into main May 5, 2025
3 of 4 checks passed
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.

1 participant