Skip to content

Conversation

FranzBusch
Copy link
Member

No description provided.

@@ -13,7 +13,7 @@
//===----------------------------------------------------------------------===//
#if os(Windows)
import ucrt
#elseif os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
#elseif canImport(Darwin)
import Darwin
#elseif os(Linux) || os(Android)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to keep the elseif here?

I think we should standardise on a pattern where possible. In other places we have:

#if canImport(Darwin)
import Darwin
#else
import Glibc
#endif

In other places I've seen #elseif canImport(Glibc)

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is all over the place and IMO something to tackle separately.

@o-nnerb
Copy link

o-nnerb commented Jun 22, 2023

There are still some places where you can change to canImport(Darwin).

I was updating my fork for a contribution, but I saw your PR. There are same statements about os restriction that are in different orders.

  1. os(iOS) || os(macOS) || os(tvOS) || os(watchOS)
  2. os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
  3. os(macOS) || os(iOS) || os(watchOS) || os(tvOS)

And there are statements like this !os(iOS) && !os(tvOS) && !os(watchOS) where you can updates to !canImport(Darwin) || os(macOS).

Nice job! I didn't think refactoring in this way would give the same results as adding || os(xrOS) 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants