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
support sdk version wildcard format #106
Changes from 6 commits
a7eee85
0026abd
4224367
6926a46
0cb0bb3
60b915e
358531f
22f2a70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,25 @@ jobs: | |
- uses: ./ | ||
with: | ||
sdk: ${{ matrix.sdk }} | ||
architecture: ia32 | ||
|
||
- name: Run hello world | ||
run: | | ||
echo "main() { print('hello world'); }" > hello.dart | ||
dart hello.dart | ||
- run: dart --version | ||
|
||
# Test using wildcard versions for the sdk parameter. | ||
test_release_wildcards: | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
sdk: [2.19.x, 3.1.x] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I haven't. We had a feature request for We could also consider just
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like most setup- actions support: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sortie - good point about the caret syntax above. I'd be fine w/ either I'll re-work this PR to use |
||
fail-fast: false | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- uses: ./ | ||
with: | ||
sdk: ${{ matrix.sdk }} | ||
|
||
- name: Run hello world | ||
run: | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
(btw, you can make in-line suggested edits to PRs using the 'add a suggestion' action)