Add Linux support #9
Conversation
dfed
force-pushed
the
dfed--linux-support
branch
2 times, most recently
from
f0c90b3 to
4e19b63
Compare
dfed
force-pushed
the
dfed--linux-support
branch
from
a06486e to
9d4225d
Compare
| uses: actions/checkout@v3 | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
This isn't a directly related change but I figured I'd bump while I was here.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9 +/- ##
==========================================
- Coverage 97.63% 97.47% -0.17%
==========================================
Files 37 37
Lines 7324 7333 +9
==========================================
- Hits 7151 7148 -3
- Misses 173 185 +12
|
dfed
force-pushed
the
dfed--linux-support
branch
2 times, most recently
from
29dd651 to
9fa58d4
Compare
| @@ -54,7 +54,7 @@ public final class DependencyTreeGenerator { | |||
| for try await generatedRoot in taskGroup { | |||
| generatedRoots.append(generatedRoot) | |||
| } | |||
| return generatedRoots.sorted().joined(separator: "\n\n").trimmingCharacters(in: .whitespacesAndNewlines) | |||
There was a problem hiding this comment.
trimmingCharacters exists on NSString, which doesn't exist in Swift on Linux. Turns out we didn't need this call anyways though.
| enum DependencyTreeGeneratorError: Error, CustomStringConvertible { | ||
| private enum DependencyTreeGeneratorError: Error, CustomStringConvertible { |
There was a problem hiding this comment.
not a necessary change but noticed that I'd forgotten to make this private while I was here.
| @@ -121,7 +121,7 @@ public enum TypeDescription: Codable, Hashable, Comparable, Sendable { | |||
| case let .closure(arguments, isAsync, doesThrow, returnType): | |||
| return "(\(arguments.map { $0.asSource }.joined(separator: ", ")))\([isAsync ? " async" : "", doesThrow ? " throws" : ""].filter { !$0.isEmpty }.joined()) -> \(returnType.asSource)" | |||
| case let .unknown(text): | |||
| return text.trimmingCharacters(in: .whitespacesAndNewlines) | |||
There was a problem hiding this comment.
since trimmingCharacters doesn't exist on Linux, we now initialize this type with a trimmedDescription which does effectively the same thing (and we've got a test to prove it!)
| "SafeDICore", | ||
| "ZippyJSON", |
There was a problem hiding this comment.
ZippyJSON does not support Linux (or technically watchOS, visionOS, and a few other platforms)
| @@ -476,9 +476,8 @@ extension TypeSyntax { | |||
| returnType: typeIdentifier.returnClause.type.typeDescription) | |||
|
|
|||
| } else { | |||
| assertionFailure("TypeSyntax of unknown type. Defaulting to `description`.") | |||
There was a problem hiding this comment.
We haven't gotten any value out of this assertionFailure in multiple years of development on this type across multiple projects. Time to remove.
| } catch { | ||
| didThrow = true | ||
| XCTAssertEqual((error as CustomStringConvertible).description, errorDescription, line: line) | ||
| XCTAssertEqual("\(error)", errorDescription, line: line) |
There was a problem hiding this comment.
This change was required to get tests working on Linux. (error as CustomStringConvertible).description was not finding our var description: String code on Linux for unknown reasons.
dfed
force-pushed
the
dfed--linux-support
branch
from
a28ae2f to
972e650
Compare
|
Merging this since we aren't really changing functionality, and we have good test coverage on the changed code. As always, happy to take feedback post merge. |
We're failing to build on Linux today. This PR gets us working on Linux by making ZippyJSON a platform-conditioned dependency, as well as fixing a host of other issues. I added Linux checks to CI as well.
When we merge, I'll need to add the linux check as required on all PRs.