Repository navigation
[Diagnostics] Use imperative msg for protocol conformance & switch-case fixits - #67909
Conversation
ktoso
left a comment
ktoso
left a comment
There was a problem hiding this comment.
Yeah given the discussion in the issue I think moving to imperative form is good
|
@swift-ci please smoke test |
…se fixits This commit changes fixit messages from a question/suggestion to an imperative message for protocol conformances and switch-case. Addresses swiftlang#67510.
9f3b224 to
8e2e625
Compare
|
Previous CI run had some failing tests - addressed them with a new commit. @ktoso |
|
Thanks, let's give this a full run to be sure that's everything covered |
|
@swift-ci please test |
|
@ktoso - Linux build is failing because of this - https://github.com/apple/sourcekit-lsp/blob/6d3db6d0850d53d9521374c1c18212d6834ae1f9/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift#L121. I can open a PR for that repo in parallel. |
|
And the macOS build is failing on |
|
@swift-ci please test |
|
The windows had succeeded earlier but timed out this time 😞 I think Linux and macOS builds are expected to fail because they are running tests that are in a different repo - https://github.com/apple/sourcekit-lsp/blob/6d3db6d0850d53d9521374c1c18212d6834ae1f9/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift#L121. What is the best way to address this? One way is to create a parallel PR that changes the |
That is indeed the way right now 😅. We can do cross-PR testing here to check the combined PRs. |
This commit corresponds to changes introduced in swiftlang/swift#67909.
How can that be done? I created swiftlang/sourcekit-lsp#799 in parallel but both need to be built together. Another way is to avoid running the |
|
@swift-ci please test |
|
Can the windows build be re-triggered? All other builds succeeded. |
|
@swift-ci please test Windows platform |
|
@bnbarham Thanks for triggering the CI. The builds succeeded - please merge both the PRs when you get a chance, thanks! |
|
Thank you for your contribution @nishithshah2211 |
|
Thanks for the merge! |
The Fix-It message that we are testing here has changed in swiftlang/swift#67909. Accept both the old and the new message to make sure SourceKit-LSP tests also pass with older toolchains (e.g. if are using Xcode 15.0, which doesn’t contain the change yet). rdar://116706363
The Fix-It message that we are testing here has changed in swiftlang/swift#67909. Accept both the old and the new message to make sure SourceKit-LSP tests also pass with older toolchains (e.g. if are using Xcode 15.0, which doesn’t contain the change yet).
The Fix-It message that we are testing here has changed in swiftlang/swift#67909. Accept both the old and the new message to make sure SourceKit-LSP tests also pass with older toolchains (e.g. if are using Xcode 15.0, which doesn’t contain the change yet). rdar://116706363
6 participants
This commit changes fixit messages from a question/suggestion to an imperative message for protocol conformances and switch-case. The only source change is in
DiagnosticsSema.def. Rest of the changes are in tests to get them to pass.Resolves #67510.