Skip to content
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

Allow the next semver major versions of shelf_web_socket and web_socket_channel #2204

Merged

Conversation

brianquinlan
Copy link
Contributor

@brianquinlan brianquinlan commented Apr 5, 2024

This is part of a plan to simplify the web_socket_channel implementation


  • 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.

@jakemac53
Copy link
Contributor

jakemac53 commented Apr 5, 2024

These versions do not yet look to be published, can we wait to land this until we are sure we are actually compatible?

If you just need to get a version solve in those packages tests, I would add a dependency override on this package.

Or, we could add a git dependency override in this package, to run the tests with the latest versions of those packages, but that isn't ideal because those packages could still change (as they haven't yet been published). So I would prefer the first option, and then publishing here once those are actually available and we can run with the newer versions on CI.

@brianquinlan
Copy link
Contributor Author

@jakemac53

These versions do not yet look to be published, can we wait to land this until we are sure we are actually compatible?

If you just need to get a version solve in those packages tests, I would add a dependency override on this package.

Or, we could add a git dependency override in this package, to run the tests with the latest versions of those packages, but that isn't ideal because those packages could still change (as they haven't yet been published). So I would prefer the first option, and then publishing here once those are actually available and we can run with the newer versions on CI.

I probably don't understand some aspect of version solving, which is why I was using this approach.

package:test and package:shelf_web_socket depend on each other.

If I want to change the semver major version of package:shelf_web_socket (e.g. in this PR), how can I use dependency overrides to allow its tests to run? If I change self_web_socket/pubspec.yaml to include:

dependency_overrides:
  shelf_web_socket: ^2.0.0-wip

Then I get an error:

dart pub get --no-example
Resolving dependencies...
Error on line 25, column 3 of pubspec.yaml: A package may not list itself as a dependency.
   ╷
25 │   shelf_web_socket: ^2.0.0-wip
   │   ^^^^^^^^^^^^^^^^
   ╵
exit code 65

Any help would be appreciated!

@jakemac53
Copy link
Contributor

@brianquinlan What you should do is override the version of package:test, in the shelf_web_socket etc packages:

dependency_overrides:
  test: 1.25.2

You get a version solve error today because there is no version of test which declares compatibility with your local package, so overriding the version of package:test is what will fix that.

Then your local tests can run, and once you publish we can bump the constraints here.

@jakemac53
Copy link
Contributor

Note that it is perfectly safe to publish with overridden dev dependencies, they do not affect your users (and in general, dependency_overrides do not affect your users, they are local only).

@brianquinlan
Copy link
Contributor Author

Note that it is perfectly safe to publish with overridden dev dependencies, they do not affect your users (and in general, dependency_overrides do not affect your users, they are local only).

Thanks!

@brianquinlan brianquinlan marked this pull request as ready for review April 11, 2024 21:16
@brianquinlan
Copy link
Contributor Author

The plan is to release the following in rapid succession:

  • package:web_socket_channel 3.0: removes WebSocketChannel constructor
  • package:shelf_web_socket 2.0: doesn't use WebSocketChannel constructor, allow web_socket_channel >= 2 < 4
  • package:test 1.26: allow web_socket_channel >= 2 < 4, allow shelf_web_socket >= 1 < 3

Does this package autopublish? I don't understand the validation failure.

@jakemac53
Copy link
Contributor

Does this package autopublish? I don't understand the validation failure.

It publishes based on tags - but because of our pinned deps we have to ignore the pub warnings by adding a label to PRs to make the validation step pass (I just did that though)

@brianquinlan
Copy link
Contributor Author

Does this package autopublish? I don't understand the validation failure.

It publishes based on tags - but because of our pinned deps we have to ignore the pub warnings by adding a label to PRs to make the validation step pass (I just did that though)

Thanks!

Would it be possible to merge this and then do a release on Monday after I've release package:web_socket_channel and package:shelf_web_socket?

@jakemac53
Copy link
Contributor

Would it be possible to merge this and then do a release on Monday after I've release package:web_socket_channel and package:shelf_web_socket?

We would want to do the publish immediately after the merge - otherwise it puts us in a weird state.

@brianquinlan
Copy link
Contributor Author

Right. So when is a good time for me to publish shelf_web_socket and web_socket_channel such that we can merge this and do a release quickly after?

If I release those packages now, can we release package:test in the next day or so?

@jakemac53
Copy link
Contributor

Right. So when is a good time for me to publish shelf_web_socket and web_socket_channel such that we can merge this and do a release quickly after?

If I release those packages now, can we release package:test in the next day or so?

Monday would probably be good, cc @natebosch

@natebosch
Copy link
Member

A publish on Monday SGTM.

These have all been running in google3 right?

@brianquinlan
Copy link
Contributor Author

Yes. They have been

A publish on Monday SGTM.

These have all been running in google3 right?

Yes, for almost 3 weeks now.

@brianquinlan
Copy link
Contributor Author

I'm going to publish package:web_socket_channel and package: shelf_web_socket now.

@jakemac53
Copy link
Contributor

@brianquinlan sgtm

@brianquinlan
Copy link
Contributor Author

I'm going to publish package:web_socket_channel and package: shelf_web_socket now.
@brianquinlan sgtm

Done. Could someone merge this PR and publish package:test?

@jakemac53
Copy link
Contributor

It looks like this got in a bit of a weird state, I can push a commit to try and fix it

@jakemac53
Copy link
Contributor

We are picking up the latest versions in the tests so that is good 👍

@brianquinlan
Copy link
Contributor Author

We are picking up the latest versions in the tests so that is good 👍

And the tests are passing, which is even better 🎉

@brianquinlan
Copy link
Contributor Author

Can we submit and release?

@jakemac53 jakemac53 merged commit d1e2767 into dart-lang:master May 6, 2024
33 checks passed
@jakemac53
Copy link
Contributor

Done and done!

@brianquinlan
Copy link
Contributor Author

Thanks so much!

copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request May 13, 2024
…erver, http_parser, json_rpc_2, markdown, matcher, mockito, package_config, path, pool, pub_semver, shelf, test, test_descriptor, test_process, test_reflective_loader, web, web_socket_channel, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/6e9c1ea..2e706be):
  2e706be8  2024-05-10  Sam Rawlins  Fix bug around ref resolution of type mentioned in type arguments (dart-lang/dartdoc#3768)
  d4a5f0b3  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/dartdoc#3766)

ecosystem (https://github.com/dart-lang/ecosystem/compare/9ce560b..5890417):
  5890417  2024-05-10  Parker Lougheed  Update dart_flutter_team_lints to depend on lints v4 (dart-lang/ecosystem#263)

fixnum (https://github.com/dart-lang/fixnum/compare/2c9f25d..ac892ad):
  ac892ad  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/fixnum#128)

glob (https://github.com/dart-lang/glob/compare/44e8c22..ee48ea8):
  ee48ea8  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/glob#93)
  c6362fb  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/glob#92)

html (https://github.com/dart-lang/html/compare/44613e8..00d3461):
  00d3461  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/html#245)
  e676b96  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/html#244)

http (https://github.com/dart-lang/http/compare/dd31e64..fdb0d36):
  fdb0d36  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/http#1194)

http_multi_server (https://github.com/dart-lang/http_multi_server/compare/e45a674..4a791af):
  4a791af  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/http_multi_server#69)
  ff49aed  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/http_multi_server#68)

http_parser (https://github.com/dart-lang/http_parser/compare/ad05810..702698a):
  702698a  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/http_parser#91)
  d900d8e  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/http_parser#90)

json_rpc_2 (https://github.com/dart-lang/json_rpc_2/compare/7547bb1..3187f7b):
  3187f7b  2024-05-08  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/json_rpc_2#114)
  94b6c49  2024-05-08  Devon Carew  blast_repo fixes (dart-archive/json_rpc_2#113)

markdown (https://github.com/dart-lang/markdown/compare/6aaa152..7463999):
  7463999  2024-05-07  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/markdown#610)
  a2313a5  2024-05-07  Devon Carew  blast_repo fixes (dart-lang/markdown#609)

matcher (https://github.com/dart-lang/matcher/compare/b42bf10..4ac4096):
  4ac4096  2024-05-07  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/matcher#248)
  f22df8a  2024-05-07  Devon Carew  blast_repo fixes (dart-archive/matcher#247)

mockito (https://github.com/dart-lang/mockito/compare/81ecb88..4be52e1):
  4be52e1  2024-05-07  Devon Carew  blast_repo fixes (dart-lang/mockito#749)

package_config (https://github.com/dart-lang/package_config/compare/a36e496..3909676):
  3909676  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/package_config#154)
  ca40813  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/package_config#153)

path (https://github.com/dart-lang/path/compare/f411b96..9be79e7):
  9be79e7  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/path#164)
  283f31a  2024-05-09  Devon Carew  blast_repo fixes (dart-archive/path#163)

pool (https://github.com/dart-lang/pool/compare/e6df05a..1a6f2df):
  1a6f2df  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/pool#86)
  f2f8a8b  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/pool#85)

pub_semver (https://github.com/dart-lang/pub_semver/compare/df834e1..f57c9c3):
  f57c9c3  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/pub_semver#103)
  2071731  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/pub_semver#102)

shelf (https://github.com/dart-lang/shelf/compare/c8d8058..d9f82bf):
  d9f82bf  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/shelf#431)
  96d7283  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/shelf#430)
  468e2d9  2024-05-06  Brian Quinlan  Remove `dependency_overrides` (dart-lang/shelf#429)
  9e57c33  2024-05-06  Brian Quinlan  Prepare shelf_web_socket 2.0 for release (dart-lang/shelf#425)

test (https://github.com/dart-lang/test/compare/0562baf..84d2a2b):
  84d2a2bc  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test#2220)
  b7fa27d5  2024-05-07  Nate Bosch  Point README links to the test library (dart-lang/test#2219)
  d1e27677  2024-05-06  Brian Quinlan  Allow the next semver major versions of shelf_web_socket and web_socket_channel (dart-lang/test#2204)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/6695954..d61bf6c):
  d61bf6c  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_descriptor#66)
  2861ba3  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_descriptor#65)

test_process (https://github.com/dart-lang/test_process/compare/69c6894..4ab3f1c):
  4ab3f1c  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_process#58)
  6efaf7f  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_process#57)

test_reflective_loader (https://github.com/dart-lang/test_reflective_loader/compare/537ab5b..f8807e0):
  f8807e0  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_reflective_loader#62)
  1c62cd9  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_reflective_loader#61)

web (https://github.com/dart-lang/web/compare/ff7185c..c36d962):
  c36d962  2024-05-12  mnordine  Fix typo (dart-lang/web#236)

web_socket_channel (https://github.com/dart-lang/web_socket_channel/compare/d86313d..5f65bae):
  5f65bae  2024-05-06  Brian Quinlan  Remove dependency_overrides (dart-lang/web_socket_channel#354)
  324d932  2024-05-06  Brian Quinlan  Make shelf_web_socket a dev dependency (dart-lang/web_socket_channel#353)
  0eca60d  2024-05-06  Brian Quinlan  Remove "wip" label for 3.0 release (dart-lang/web_socket_channel#347)

webdev (https://github.com/dart-lang/webdev/compare/c233e45..9108903):
  91089031  2024-05-10  Elliott Brooks  Update CONTRIBUTING doc for DWDS (dart-lang/webdev#2429)
  635b0d9d  2024-05-10  Elliott Brooks  Delete MV2 Dart Debug Extension (dart-lang/webdev#2428)
  ae43e8e9  2024-05-09  Elliott Brooks  Remove all unsound tests (dart-lang/webdev#2426)
  239cb7f3  2024-05-08  Elliott Brooks  Register service extensions on a client that is connected to DDS  (dart-lang/webdev#2388)

Change-Id: Ic0de7f6e41b4e93e3d26b7d1a74fe012bc7b086e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366161
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants