-
Notifications
You must be signed in to change notification settings - Fork 214
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
Update the use of 'package:shelf_web_socket's webSocketHandler
method
#2421
Conversation
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
We should be able to land this change without widening the constraint pre-emptively, and then in shelf_web_socket we can add a dependency override on the latest version of package:test (with the forwards compatible change). Then, when shelf_web_socket is published, we can update the constraint here and publish again (after which point the override in shelf_web_socket can be removed). That avoids publishing any regular dependency constraints in any packages with versions that aren't actually there yet (it will only be an override on the dev dependency in shelf_web_socket, which is fine). WDYT? |
Sounds great! Forward declaring the dep - even for something we control - does feel like crossing your fingers and hoping. |
webSocketHandler
method
Co-authored-by: Jacob MacDonald <jakemac@google.com>
Is it expected here that the publish check fails? We just ignore for this package, but still use publishing automation after this lands?
|
Yes, we have to add the label to ignore warnings because we pin test_core and test_api. |
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
lints (https://github.com/dart-lang/lints/compare/2e1321e..e1d4794): e1d4794 2024-12-03 Michael Goderbauer Revert "Update README.md before archiving (`#214`)" (dart-lang/lints#219) shelf (https://github.com/dart-lang/shelf/compare/657ebd3..2b5b683): 2b5b683 2024-12-02 Devon Carew move analysis_options.yaml into packages (dart-lang/shelf#460) sse (https://github.com/dart-lang/sse/compare/befbd6d..b97dc3a): b97dc3a 2024-12-02 dependabot[bot] Bump dart-lang/setup-dart in the github-actions group (dart-lang/sse#119) test (https://github.com/dart-lang/test/compare/c2a6986..2096773): 20967732 2024-12-03 Devon Carew Update the use of 'package:shelf_web_socket's `webSocketHandler` method (dart-lang/test#2421) 1d28d738 2024-12-02 Ben Konyi Support `package:vm_service` 15.x (dart-lang/test#2420) Change-Id: I852515f46f12b53d2a2fe665a57f7ddcfde36222 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398605 Commit-Queue: Devon Carew <devoncarew@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
webSocketHandler
methodThis will allow us to add more type info to the closure that
webSocketHandler
expects; it's currently an untyped Function. See also dart-lang/shelf#457 and dart-lang/shelf#463.This forward declares compatibility with
3.0
ofpackage:shelf_web_socket
; I think this is necessary - asdart test
uses both package:test and package:shelf_web_socket - but happy to hear otherwise.Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.