Skip to content

Conversation

@kpsroka
Copy link

@kpsroka kpsroka commented Aug 14, 2024

  • Thanks for your contribution! Please replace this text with a description of what this PR is changing or adding and why, list any relevant issues, and review the contribution guidelines below.

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@github-actions github-actions bot added the package:cupertino_http Issues related to package:cupertino_http label Aug 14, 2024
@kpsroka
Copy link
Author

kpsroka commented Aug 15, 2024

Hi @brianquinlan,
This is the approach I want to take wrt #1294. Let me know if that looks fine to you, and I'll continue making this change prod-ready.

@kpsroka kpsroka changed the title [DRAFT] Allows native implementations to use close codes from RFC 6455 [DRAFT] Allows native WebSocket implementations to use close codes from RFC 6455 Aug 15, 2024
Copy link
Collaborator

@brianquinlan brianquinlan left a comment

Choose a reason for hiding this comment

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

Looks good to me! Let me know if I should re-run the checks.

/// Throws an [ArgumentError] if [code] is not 1000 or in the range 3000-4999.
/// Throws an [ArgumentError] if [code] is not accepted by the implementation.
/// All implementations must accept [code] values of 1000, and in the range
/// 3000-4999. Some implementations may accept additional values, but must not
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe change must => will because this is meant to be read by a WebSocket user not implementer ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package:cupertino_http Issues related to package:cupertino_http

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants