[Collections] Tighten import conditions #3296
Conversation
yim-lee
requested review from
abertelrud,
friedbunny,
neonichu and
tomerd
as code owners
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
| @@ -1224,6 +1328,10 @@ final class PackageCollectionsTests: XCTestCase { | |||
| try XCTSkipIf(true) | |||
| #endif | |||
|
|
|||
| if !PackageCollections.isSupportedPlatform { | |||
| try XCTSkipIf(true) | |||
There was a problem hiding this comment.
try XCTSkipIf(!PackageCollections.isSupportedPlatform)
There was a problem hiding this comment.
I think there is a way in XCTest to apply such rules to all tests in the class, which may be easier. @neonichu do you remember how?
There was a problem hiding this comment.
Not aware of anything tbh
| @@ -14,6 +14,13 @@ import TSCBasic | |||
|
|
|||
| // TODO: is there a better name? this conflicts with the module name which is okay in this case but not ideal in Swift | |||
| public struct PackageCollections: PackageCollectionsProtocol { | |||
| // Check JSONPackageCollectionProvider.isSignatureCheckSupported before updating or removing this | |||
There was a problem hiding this comment.
I wonder if there is a way to exclude the entire PackageCollections module in unsupported platforms. not sure if easier / harder but if easy may hep to keep the code simpler and move the complexity there
There was a problem hiding this comment.
cc @neonichu may know of a way
There was a problem hiding this comment.
In principle, we could use conditional dependencies, might need some reorganization because I think right now some of the collections targets are also directly included in products.
|
looks good, some ideas/thought on how to simplify this but its good either way. |
tomerd
added
the
ready
yim-lee
force-pushed
the
security-import
branch
from
8a74a06 to
51dd952
Compare
|
@swift-ci please smoke test |
yim-lee
force-pushed
the
security-import
branch
from
51dd952 to
6559f39
Compare
|
@swift-ci please smoke test |
Some `Security` framework APIs are available on macOS only
yim-lee
force-pushed
the
security-import
branch
from
6559f39 to
c3089f9
Compare
|
@swift-ci please smoke test |
Some
Securityframework APIs are available on macOS only