Drop support for Node < 10 and Ember < 2.18.#73
Drop support for Node < 10 and Ember < 2.18.#73rwjblue merged 7 commits intoember-fastboot:masterfrom
Conversation
|
See https://github.com/chancancode/ember-cli-head/pull/1/checks for the GitHub Actions runs |
chancancode
force-pushed
the
upgrades
branch
4 times, most recently
from
f715298 to
bc529ca
Compare
| if: matrix.scenario != 'default-with-fastboot' | ||
| # Due to a bug in ember-cli, running `ember test` with `--path` doesn't set `EMBER_ENV=test` | ||
| # See https://github.com/ember-cli/ember-cli/issues/8922 | ||
| run: EMBER_ENV=test yarn test --path dist |
There was a problem hiding this comment.
Wouldn't that run ember test twice for each scenario, once with the ember-try scenario's dependencies (L47), and once with the default ones here? Also linting for each scenario.
I tried to replicate the default Travis setup as much as possible, have one job that runs linting and default tests, one with floating deps, and then the matrix of ember-try scenarios. E.g. like this: https://github.com/kaliber5/ember-in-element-polyfill/blob/master/.github/workflows/ci.yml
There was a problem hiding this comment.
L47 only does the yarn install and sets the optional feature ENV variable, because I passed a command to ember try instead of the default (ember test). I like to break down the steps as much as possible so it makes it easier to see what failed and find the logs.
As for floating dependencies, all of the scenarios are floating by default (—no-lockfile) and I added one that tests with lockfile specifically. This actually confused me a little because the default Travis yaml also uses —no-lockfile by default so I don’t actually understand why the floating dependencies one is needed at all.
|
Seems this is now configured to throw on deprecations, so canary is failing due to |
|
Yep exactly. But since none of the checks are required (or even running on this repo), we can probably just merge this and rebase #71 on top. Would be a good excuse to test out the new CI setup in the context of a PR. |
| headElement: computed(function() { | ||
| let documentService = getOwner(this).lookup('service:-document'); | ||
| return documentService.head; | ||
| }), |
There was a problem hiding this comment.
Is this removed because it is not needed anymore?
There was a problem hiding this comment.
This was moved to init. Some tests is overwriting this and it was causing the computed clobbering deprecation.
| if: matrix.scenario != 'default-with-lockfile' | ||
| run: yarn install --non-interactive --no-lockfile | ||
| - name: Setup ember-try scenario | ||
| if: matrix.scenario != 'default' && matrix.scenario != 'default-with-lockfile' |
There was a problem hiding this comment.
p1/5
FWIW, I find this style significantly harder to read/understand (vs making a different jobs). I'm curious why you prefer it?
There was a problem hiding this comment.
Mostly because they seem to share a lot of the steps. GH Actions have very limited abstraction/reuse features, so it kind of sucks either way. I am pretty indifferent, if you prefer, I can change it.
4 participants
{{#in-element}}(from emberjs/rfcs#287) #71)See https://github.com/chancancode/ember-cli-head/pull/1/checks for the GitHub Actions runs. They should start showing up here once this lands. Canary tests are currently failing, should be fixed by #71.