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

Pin CI to latest stable Dart 2.10 & prepare for NNBD #276

Merged
merged 5 commits into from
Dec 24, 2020

Conversation

Liklainy
Copy link
Contributor

@Liklainy Liklainy commented Dec 8, 2020

  • Added matrix with latest and dev tags
  • Disabled fail-fast for now

Closes actions/toolkit#270

.github/workflows/test.yml Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@Stargator
Copy link
Contributor

@Liklainy I just updated the fetch_configlet to hopefully address the exit code issue. Please update your master branch and then issue-270 onto master. Let's see if that fixes that

@Stargator
Copy link
Contributor

Stargator commented Dec 12, 2020

@Liklainy Can you update the Install dependencies step to use pub upgrade only when building for dev?

I think that could resolve some of the errors due to dependencies not being null-safe.

@devkabiir
Copy link
Contributor

devkabiir commented Dec 13, 2020

Since dart 2.10 we have the new unified dart tool, so instead of using pub upgrade we should be using dart pub upgrade
See: https://medium.com/dartlang/announcing-dart-2-10-350823952bd5

@Liklainy
Copy link
Contributor Author

  • Added 2.10 tag
  • Changed CI to use dart tool
  • Changed fail-fast: false to continue-on-error only for dev tag
    So now only if dev job failed, workflow will continued and will be shown as successfull
    But it's not like that in every place, for example in commit it shows ❌, there is an issue Please support something like "allow-failure" for a given job actions/runner#2347 which asks allow-failure implementation or fixing continue-on-error to behave like allow-failure

@Stargator, dev with upgrade is successfully passed
I'm not sure that upgrade should be in CI, with it we wouldn't see the need to upgrade packages for dev
Removed upgrade in next commit to test continue-on-error, will add it back if it's needed

Should I squash commits?

@Stargator
Copy link
Contributor

Stargator commented Dec 15, 2020

@Liklainy and @devkabiir I ran `dart pub outdated --mode=null-safety and got the following result.

This was after upgrading the test and yaml packages to their null safe versions.

Screen Shot 2020-12-15 at 7 21 01 AM

Given this result and that I had to manually change the sdk version constraint to sdk: '>=2.12.0-0 <3.0.0' just to properly use the dev version makes me believe there's no way we can resolve errors with Dart dev until more packages migrate.

Additionally, I had to set the versioning for the test and yaml packages above their latest versions in order to upgrade to their null safe versions.

  test: '>=1.15.8 <2.0.0'
  yaml: '>=2.3.0 <4.0.0'

So for the sake of a stable CI process, for now, I recommend dropping dev. That way we won't always have something failing. @devkabiir what do you think?

For me, that means we keep the matrix setup and can even keep the 2.10 and latest tags if @devkabiir wants. So we at least have it setup to add dev when we can.

@devkabiir
Copy link
Contributor

devkabiir commented Dec 19, 2020

So for the sake of a stable CI process, for now, I recommend dropping dev. That way we won't always have something failing. @devkabiir what do you think?

I agree.

For me, that means we keep the matrix setup and can even keep the 2.10 and latest tags if @devkabiir wants. So we at least have it setup to add dev when we can.

Keep the matrix setup with only 2.10 tag since the latest is redundant at this point and as evident by https://groups.google.com/a/dartlang.org/g/misc/c/GMHD6LOLJ1I/m/2hPXC5muAwAJ,
There won't be a Dart 2.11 so when 2.12 comes we will have to do migration. Keeping the matrix setup with only 2.10 allows for stable CI as well as prepares for next CI update.

Should I squash commits?

Don't worry about that, I'd prefer to use Squash & Merge strategy for this PR, so the resultant commit will have a sensible name and PR link automatically added by GitHub, and we can also choose to include individual commit messages in merge commit's body. (Squash & Merge results in a single commit)

@devkabiir devkabiir changed the title Add dev container to CI Pin CI to latest stable Dart 2.10 & prepare for NNBD Dec 19, 2020
@devkabiir
Copy link
Contributor

@Liklainy I appreciate your patience and continued efforts towards this PR very much. 🚀

@Liklainy
Copy link
Contributor Author

Current changes in this PR:

  • Only 2.10 tag
    • dev and latest dropped
  • matrix setup with fail-fast disabled for future tags
    • matrix for fast adding next tags when everyhing is ready
    • fail-fast to allow all tags to run without cancelling in the middle when another one is failed, to see clearly when anything is working in one version and failing in another
  • Using dart tool in CI

@Liklainy Liklainy requested a review from devkabiir December 24, 2020 14:27
Copy link
Contributor

@devkabiir devkabiir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution 🚀.

PS: Thanks for the ping.

@devkabiir devkabiir merged commit 476e850 into exercism:master Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update GitHub Action: Build/test against latest stable version of Dart and dev version
3 participants