support sdk version wildcard format #106
Conversation
|
@sortie - do you mind reviewing this? |
.github/workflows/dart.yml
Outdated
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| sdk: [2.19.x, 3.1.x] |
There was a problem hiding this comment.
Have you considered using ^2.19.0 instead? Then this would be more similar to how you specify SDK constraints, maybe we could even support more complicated SDK constraints in the future?
There was a problem hiding this comment.
Hmm, I haven't. We had a feature request for 2.19.x, and I believe this may match the action support for one other language.
We could also consider just 2.19; using just two version segments could be an implicit way to opt-into using the latest patch release for a version.
^2.19.0 could also work. The current way this PR is implemented, we couldn't efficiently have an actual semver match - like getting the latest version that matches a version range (i.e., '>=2.18.3 <2.19.2', ...). We could do that if we have a json method we could call that either returned all the available sdk versions, or, all the versions that matched a given range. We're currently relying on a very specific usage of the google storage list API. It works the way we want it for this specific use case, but for anything more sophisticated we'd need to have all the sdk versions available (perhaps read from a single well-known file in google storage - https://storage.googleapis.com/dart-archive/channels/stable/release/versions.json?).
There was a problem hiding this comment.
Up to you :)
Either option will work. Caret syntax feels more "Dart". Not sure if we ever need more complicated constraints (definitely wouldn't implement it now without a use case).
There was a problem hiding this comment.
We could also just have e.g. "2.19" to select the latest 2.19.x release
Agreed that caret syntax feels dart-y although it also does suggest the powerful of the whole pub version resolution
There was a problem hiding this comment.
The caret is actually kinda bad since ^3.1.0 means up to 4.0, which isn't the semantics we try to do here.
There was a problem hiding this comment.
That's true, caret means until the next major, so it doesn't quite cover the use case of latest patch version. So for that we'd need to support pub-like version ranges which seems inconvenient. So I think rather than making this too complicated we should look at other languages and make a "GitHub-y" solution.
There was a problem hiding this comment.
Seems like most setup- actions support: 3, 3.2, 3.4.5. The .x is probably supported as well mostly because https://github.com/npm/node-semver supports it.
There was a problem hiding this comment.
@sortie - good point about the caret syntax above.
I'd be fine w/ either 2.19 or 2.19.x. They're both simple. If the two version variant (2.19) is more common for github actions then we should just do that. We could support both patterns but I don't think the flexibility is really necessary here.
I'll re-work this PR to use 2.19.
|
I updated the PR to use |
.github/workflows/dart.yml
Outdated
| - run: dart --version | ||
|
|
||
| # Test using wildcard versions for the sdk parameter. | ||
| test_release_wildcards: |
There was a problem hiding this comment.
Nit: test_latest_patch_release (and # Test getting the latest patch release for a major.minor sdk parameter.)
There was a problem hiding this comment.
done!
(btw, you can make in-line suggested edits to PRs using the 'add a suggestion' action)
| final versionPrefix = sdk.substring(0, sdk.length - '.x'.length); | ||
| version = await findLatestSdkForRelease(versionPrefix); | ||
| // Find the latest version for the given sdk release. | ||
| version = await findLatestSdkForRelease(sdk); |
There was a problem hiding this comment.
How about the same but for beta releases? E.g. the latest version in a beta series? Or maybe 3.3.0-1.beta would give you 3.3.0-1.1.beta and later 3.3.0-1.2.beta and onwards.
Although in those cases you can kinda just follow the beta channel, although that would give potential breakages whenever a new beta series is dropped, whereas a patched beta is only more stable.
There was a problem hiding this comment.
Do you mean something like 3.1-beta? I.e., the latest beta for a given sdk release?
I don't think that's a common use case. I think testing against the latest stable is common, and testing against the upcoming releases (beta, dev, ...) is common. What this PR will help with is for people that want to test against specific SDKs (for example, the lower sdk constraint of their pubspec), but want the latest patch release for that SDK. Right now they're using 3.1.0 as a stand-in for that lower bounds, but I think they really want to test against 3.1.<latest patch>.
|
Thanks for the feedback and discussion! Much appreciated. |
|
\cc @kevmoo I really like this since it's can resolve the main issue I had with the Do you think a new |
2.19.x,3.1.x, ...)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.