Skip to content

Avoid conforming String to Error in this module #229

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

Merged

Conversation

harlanhaskins
Copy link
Contributor

@harlanhaskins harlanhaskins commented Mar 26, 2022

This seems like an overly-broad conformance to add to
swift-experimental-string-processing. Do we need this?

I noticed this while testing swiftlang/swift#36068, this trips the resilient retroactive conformance warning since we're building this with library evolution.

This seems like an overly-broad conformance to add to
swift-experimental-string-processing, and it doesn't look like
anyone is calling report(). Do we need this?
@harlanhaskins harlanhaskins requested a review from milseman March 26, 2022 03:22
@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

1 similar comment
@rxwei
Copy link
Contributor

rxwei commented Mar 26, 2022

@swift-ci please test

@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

@harlanhaskins
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big picture LGTM, Hamish may want to look at the parser errors

// Errors that may be thrown from default implementations
private enum ParticipantError: Error {
case unsupported
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hamishknight can you take a look?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used for testing AFAIK

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@harlanhaskins harlanhaskins merged commit f66e8be into swiftlang:main Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants