-
Notifications
You must be signed in to change notification settings - Fork 54
Clean up unused code and replace Path with URL to resolve warnings #407
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
7588024
fc78d0c
9fd760f
3672645
64c1505
2a56d09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,7 @@ struct GenerateDocsReferencePlugin: CommandPlugin { | |
arguments: [String] | ||
) async throws { | ||
// Locate generation tool. | ||
let generationToolFile = try context.tool(named: "generate-docs-reference").path | ||
|
||
let generationToolFile = try context.tool(named: "generate-docs-reference").url | ||
// Create an extractor to extract plugin-only arguments from the `arguments` | ||
// array. | ||
var extractor = ArgumentExtractor(arguments) | ||
|
@@ -52,18 +51,20 @@ struct GenerateDocsReferencePlugin: CommandPlugin { | |
guard builtArtifact.kind == .executable else { continue } | ||
|
||
// Get the artifacts name. | ||
let executableName = builtArtifact.path.lastComponent | ||
let executableName = builtArtifact.url.lastPathComponent | ||
|
||
print("Generating docs reference for \(executableName)...") | ||
|
||
let outputFile = context.package.directory | ||
.appending("Documentation/SwiftlyDocs.docc/swiftly-cli-reference.md") | ||
let outputFile = context.package.directoryURL | ||
.appendingPathComponent("Documentation") | ||
.appendingPathComponent("SwiftlyDocs.docc") | ||
.appendingPathComponent("swiftly-cli-reference.md") | ||
|
||
// Create generation tool arguments. | ||
var generationToolArguments = [ | ||
builtArtifact.path.string, | ||
builtArtifact.url.path(), | ||
"--output-file", | ||
outputFile.string, | ||
outputFile.path(), | ||
|
||
] | ||
generationToolArguments.append( | ||
contentsOf: extractor.remainingArguments) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,7 +249,6 @@ struct InstallToolchainInfo: OutputData { | |
let versionContainer = try container.nestedContainer( | ||
keyedBy: ToolchainVersionCodingKeys.self, forKey: .version | ||
) | ||
let name = try versionContainer.decode(String.self, forKey: .name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (blocking): I think that the name could be in the decoding stream, even though it is redundant for decoding because it is a synthesized property of ToolchainVersion. I expect that the name will need to be decoded here, even if the return value is thrown away. Some kind of handling of the name key here will also help with symmetry between this init and the encode method above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your feedback. To clarify, are you expecting the decode to be strict in this context? _ = try versionContainer.decode(String.self, forKey: .name) This approach ensures that decoding fails if the name is missing, maintaining symmetry with the encode(to:) function, even if the value is not directly used. |
||
|
||
switch try versionContainer.decode(String.self, forKey: .type) { | ||
case "stable": | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I’m mistaken, this should be
path(percentEncoded: false)
.See FB20238046 and https://developer.apple.com/forums/thread/784446.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! Totally makes sense since we're dealing with file system paths