Skip to content
This repository was archived by the owner on May 29, 2025. It is now read-only.

Conversation

@wangqi
Copy link
Contributor

@wangqi wangqi commented Apr 28, 2025

…ed to make the Process-dependent functionality only available on platforms where the API exists.

Summary

This PR addresses platform compatibility issues with the Process class by using Swift's conditional compilation to make Process-dependent functionality only available on macOS. This approach leverages compile-time safety rather than runtime errors, resulting in cleaner and more maintainable code.

Details

Changes

  • Used #if os(macOS) conditional compilation directives to restrict Process-dependent code to macOS
  • Added clear documentation indicating which functions are only available on macOS
  • Removed unnecessary stub implementations for iOS

Technical Details

  • Modified Process+extensions.swift to conditionally compile Process extensions only for macOS
  • Updated DataChannel+StdioProcess.swift to wrap all Process-dependent functionality in platform-specific directives
  • Added proper documentation comments indicating platform availability restrictions
  • Kept error types accessible across platforms for consistent error handling

Copy link
Owner

@gsabran gsabran left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great! Can you revert the changes to the Package.resolved ? I don't think they are needed.

…ed to make the Process-dependent functionality only available on platforms where the API exists.
@wangqi
Copy link
Contributor Author

wangqi commented Apr 28, 2025

Hi, I’ve reverted the MCPClientChatDemo/MCPClientChat/MCPClientChat.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved file. It was automatically modified by Xcode when I was testing the build, and I didn’t notice the change. Sorry about that!

Copy link
Owner

@gsabran gsabran left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@gsabran
Copy link
Owner

gsabran commented Apr 28, 2025

No worries, swift package is a pain to work with...

@gsabran gsabran merged commit 9674388 into gsabran:main Apr 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants