-
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
use pub workspaces #2249
use pub workspaces #2249
Conversation
I don't necessarily have an answer here. But I think it's worthwhile questioning some of the decisions that led us here. If we want to check version constraints in workspace (and we do) using a Or we could use GitHub Actions to bump version numbers and publish using workflow_dispatch. If we wanted to automate some of the tedious manual bits. I don't have an answer off the top of my head, I'm just not 100% sure we shouldn't reconsider our workflow / conventions around -wip. |
It's not clear how much effort that would take. We made a lot of progress in decoupling these packages, but not enough to use a looser dependency yet.
We split the packages to allow some use cases to have pruned transitive deps, so we can't merge them without changes in pub.
It's our documented convention. https://github.com/dart-lang/sdk/blob/main/docs/External-Package-Maintenance.md#making-a-change I find it's convenient to have a quick place to check whether there are unpublished changes, and reliable enough even without enforcement. Without this we have more often made mistakes like double-incrementing the version number with a single publish. If we adopt tooling which does the edits for us we could implement it to check for published versions and warn about double incrementing maybe. I'm uneasy about dropping the practice without tooling though. |
We might consider this at this point, but it wouldn't solve the problem. Any time we have a new feature in one package, that isn't released yet, we will have to use one of these
It used to be one giant package, but was split out for good reasons (fewer transitive deps). Flutter only depends on the smaller (test_api) package for the most part (at least in terms of non-dev dependencies), which means a lot fewer pinned deps in total.
Yes, it is a good convention, and it is the general established convention for dart-lang packages. Packages are never published with that version, it just indicates "there are unpublished changes, and based on those this is the next anticipated version number". It makes it easy to tell at a glance which packages have unpublished changes and also allows you to evolve the next pending release version along with the changes actually being made.
Workspaces don't allow dependency_overrides of the packages in the workspace (this is what we previously did).
Yeah, the solution I came up with is sort of ok, not ideal but works. I prefer it to dependency overrides, because we can at least ensure our version constraints are compatible. We have before forgotten to bump pinned versions because of the overrides.
We could do that but it feels wrong to have the changelog/pubspec out of sync. It is a work in progress version, and so having that in the pubspec seems correct to me. It is also error prone to not do that. We want to be able to update the pubspec in other workspace packages to depend on WIP features (see last comment below).
Maybe, not sure exactly what this would look like.
Yeah I am not suggesting changing how the version solve works, or hard coding knowledge about
I don't see a good workflow for this today. I don't have a solution to pitch yet either, but it might be worth thinking about. |
cc @devoncarew - WDYT about adding a check in firehose for a package which has a more than +1 increment in any semver position over what is published? I can imagine edge cases with retracted packages. I'm not sure if there are other reasons to have a version bump that skips a number. |
Oh, I see Nate beat me too it :). |
Going to hold off just a bit on landing this, we have some changes that would be good to get out prior to dropping new feature support for anything <=3.4. Nate also brought up the idea via chat of just pinning the |
I think adding additional safety checks would be valuable. If they are checks that are mostly always things we want to enforce - but that have infrequent, legitimate, violations, we can either handle that via:
|
…em, file, glob, http, http_multi_server, http_parser, json_rpc_2, logging, mockito, package_config, source_maps, source_span, sync_http, test, yaml_edit Revisions updated by `dart tools/rev_sdk_deps.dart`. args (https://github.com/dart-lang/args/compare/6a5a2e6..1a24d61): 1a24d61 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-archive/args#278) bazel_worker (https://github.com/dart-lang/bazel_worker/compare/c76d7c8..02f190b): 02f190b 2024-07-01 Kevin Moore blast_repo fixes (dart-archive/bazel_worker#94) benchmark_harness (https://github.com/dart-lang/benchmark_harness/compare/f6ef33d..a06785c): a06785c 2024-07-01 Kevin Moore blast_repo fixes (dart-archive/benchmark_harness#108) collection (https://github.com/dart-lang/collection/compare/9354f38..0c1f829): 0c1f829 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-archive/collection#354) ecosystem (https://github.com/dart-lang/ecosystem/compare/54ca01a..4171189): 4171189 2024-07-02 Jacob MacDonald support nested packages in firehose (dart-lang/ecosystem#277) 459041b 2024-07-01 dependabot[bot] Bump the github-actions group with 4 updates (dart-lang/ecosystem#275) file (https://github.com/google/file.dart/compare/07cacae..855831c): 855831c 2024-07-01 dependabot[bot] Bump actions/checkout from 4.1.6 to 4.1.7 (dart-archive/file.dart#242) da79121 2024-07-01 dependabot[bot] Bump dart-lang/setup-dart from 1.6.4 to 1.6.5 (dart-archive/file.dart#241) glob (https://github.com/dart-lang/glob/compare/6d3ba5e..8b05be8): 8b05be8 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/glob#96) http (https://github.com/dart-lang/http/compare/8d89385..4178b67): 4178b67 2024-07-03 Brian Quinlan Clarify when Client.close must be called (dart-lang/http#1255) 719dc5f 2024-07-02 Brian Quinlan Upgrade to http_image_provider: 0.0.3 (dart-lang/http#1253) 75b1efb 2024-07-02 dependabot[bot] Bump the github-actions group across 1 directory with 4 updates (dart-lang/http#1251) cdfb94c 2024-07-01 Brian Quinlan Add an section explaining the benefits of using `package:ok_http`. (dart-lang/http#1252) http_multi_server (https://github.com/dart-lang/http_multi_server/compare/25941e2..8348be1): 8348be1 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-archive/http_multi_server#71) http_parser (https://github.com/dart-lang/http_parser/compare/9bf7bd9..ce528cf): ce528cf 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-archive/http_parser#101) json_rpc_2 (https://github.com/dart-lang/json_rpc_2/compare/616937f..b4810dc): b4810dc 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-archive/json_rpc_2#117) logging (https://github.com/dart-lang/logging/compare/6c3fb37..8752902): 8752902 2024-07-01 Kevin Moore blast_repo fixes (dart-archive/logging#170) mockito (https://github.com/dart-lang/mockito/compare/a7fdf71..eb4d1da): eb4d1da 2024-07-03 James Lin Update with review feedback from srawlins db19e8c 2024-06-05 James Lin Provide better documentation for `provideDummy`/`provideDummyBuilder` 330976e 2024-07-01 dependabot[bot] Bump the github-actions group across 1 directory with 2 updates (dart-lang/mockito#761) package_config (https://github.com/dart-lang/package_config/compare/903a0e5..f0b7256): f0b7256 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/package_config#156) source_maps (https://github.com/dart-lang/source_maps/compare/caa79c2..5f82c61): 5f82c61 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/source_maps#95) source_span (https://github.com/dart-lang/source_span/compare/89520f3..f81cd4a): f81cd4a 2024-07-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/source_span#115) sync_http (https://github.com/dart-lang/sync_http/compare/7622bdd..ab8377e): ab8377e 2024-07-01 dependabot[bot] Bump dart-lang/setup-dart from 1.6.2 to 1.6.5 (google/sync_http.dart#48) test (https://github.com/dart-lang/test/compare/3256c23..14f9b3e): 14f9b3ec 2024-07-08 Jacob MacDonald use pub workspaces (dart-lang/test#2249) c14ce93a 2024-07-03 Nate Bosch Prepare to publish (dart-lang/test#2250) yaml_edit (https://github.com/dart-lang/yaml_edit/compare/57a28da..d605cce): d605cce 2024-07-04 Kavisi Fix fold literal encoding with trailing line break (dart-lang/yaml_edit#91) Change-Id: I70ee32b3fa1912457b7b08affb3446523ef3b0d1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/374860 Reviewed-by: Brian Quinlan <[email protected]> Commit-Queue: Devon Carew <[email protected]>
There is one weird quirk here - our usual way of pinning package versions doesn't work, because we are pinning a version which doesn't yet exist, using the non-wip number. I resolved that by adding version constraints like this instead
>=x.x.x-wip <x.x.(x+1)
. But, it is a bit weird. When creating a release, we can change these to exact versions, because they will then match the version in the repo. But, we might not remember to. WDYT @natebosch ?Also cc @sigurdm and @jonasfj in case they have a different solution to suggest.