Skip to content

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
@ghost
Copy link

ghost 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 team@review.ai.

Copy link

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