Commands: update ArgumentParser to 1.1.4 #5884
Conversation
MaxDesiatov
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
This comment was marked as outdated.
This comment was marked as outdated.
|
@swift-ci please test with the following PRs |
|
@swift-ci please smoke test |
|
apple/swift#59009 @swift-ci smoke test |
|
@MaxDesiatov there may be a few other spots to get this this in. @shahmishal has a list |
|
apple/swift#59009 @swift-ci smoke test |
|
Not quite sure how to make CI pass here without merging the |
It's normal for the self-hosted jobs to fail for this type of change, but cross-repo testing should allow getting the smoke tests jobs to pass. Once they do, we have to merge all the linked PRs at once. |
|
BTW, from the Linux smoke test failure I would gather that the |
Hm, but self-hosted jobs are required to pass on this repo, and cross-repo testing doesn't allow changing the branch of |
Maybe there's an issue with cross-repo testing? I see in the log it checks out |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
|
apple/swift#59009 @swift-ci smoke test |
Ah yah, I guess we recently made them required, so @tomerd will need to do a force merge once we have the smoke testing jobs passing. |
|
yes, I can force merge once the only failures are the expected ones. we changed policy to do that since its more explicit and safe, as we had PRs merged with failures in self hosted which were not legitimate to merge with |
MaxDesiatov
changed the title
|
apple/swift#62033 @swift-ci smoke test |
MaxDesiatov
force-pushed
the
maxd/argument-parser-update
branch
from
a837a84
to
3e74c3f
Compare
|
apple/swift#62033 @swift-ci smoke test |
1 similar comment
|
apple/swift#62033 @swift-ci smoke test |
MaxDesiatov
force-pushed
the
maxd/argument-parser-update
branch
from
3ab1760
to
91f3075
Compare
|
apple/swift#59009 @swift-ci smoke test |
| } | ||
| } catch { | ||
| throw error | ||
| } |
There was a problem hiding this comment.
nit: this whole block can be made more concise
There was a problem hiding this comment.
You mean collapsing catch and switch into a single block? I don't think there's a way for catch to match against enum cases, is there?
There was a problem hiding this comment.
maybe something like this? I also added an XCTFail for when it does not throw since that seems to be an error case? you could also use XCTAssertThrowsError for that.
do {
let stdout = try execute(["-help"]).stdout
XCTFail("expecting an error")
} catch SwiftPMProductError.executionFailure(_, _, let stderr) {
XCTAssertMatch(stderr, .contains("Usage: swift package"))
}
|
@swift-ci please smoke test |
|
apple/swift#59009 @swift-ci smoke test |
…xd/argument-parser-update # Conflicts: # Sources/CoreCommands/Options.swift
|
apple/swift#59009 @swift-ci smoke test |
…xd/argument-parser-update # Conflicts: # Sources/CoreCommands/Options.swift
|
apple/swift#59009 @swift-ci smoke test |
1 similar comment
|
apple/swift#59009 @swift-ci smoke test |
|
@swift-ci please test Windows |
|
apple/swift#59009 @swift-ci test Windows |
…xd/argument-parser-update # Conflicts: # Sources/CoreCommands/Options.swift
|
apple/swift#59009 @swift-ci smoke test |
|
apple/swift#59009 @swift-ci smoke test |
1 similar comment
|
apple/swift#59009 @swift-ci smoke test |
Motivation:
Plenty of updates were added to
ArgumentParsersince 1.0.3. For example,@OptionGroup(_hiddenFromHelp: true)is now@OptionGroup(visibility: .hidden). Also, some of the placeholder help output is formatter in a nicer way, forswift package initin our case.Modifications:
Updated uses of
@OptionGroup(_hiddenFromHelp: true), updated a test that checks forswift package inithelp output.Result:
We can utilize new features of
ArgumentParserin some future PR if needed.