fix: align linters with recent changes in classic app blueprint #119
Conversation
|
I think the test failures are pointing out that our engine declaration is wrong 🤔 https://github.com/ember-cli/ember-app-blueprint/blob/main/files/package.json#L106 I'm pretty sure that now we are on Vite 7 we should update this as a bugfix against release |
mansona
force-pushed
the
fix/align-linters-with-classic
branch
from
60dbc5f to
8d30daf
Compare
NullVoxPopuli
force-pushed
the
fix/align-linters-with-classic
branch
from
8d30daf to
c8c533f
Compare
|
I rebased for ye |
|
current issue on non-windows: |
files/_js_eslint.config.mjs
Outdated
| 'ember-cli-build.js', | ||
| ], | ||
| ...n.configs['flat/recommended-script'], | ||
| files: ['**/*.cjs', 'config/**/*.js', 'tests/dummy/config/**/*.js', 'ember-cli-build.js'], |
There was a problem hiding this comment.
Apps don't have tests/dummy.
files/_ts_eslint.config.mjs
Outdated
| 'ember-cli-build.js', | ||
| ], | ||
| ...n.configs['flat/recommended-script'], | ||
| files: ['**/*.cjs', 'config/**/*.js', 'tests/dummy/config/**/*.js', 'ember-cli-build.js'], |
There was a problem hiding this comment.
Apps don't have tests/dummy.
| ...globals.node, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Trailing comma needs to go back (Prettier).
|
well I guess this one is superseded by #138 I fine with just closing it |
|
Not entirely, the prettier and ember-try changes are still useful. |
|
maybe I acted to soon, the important parts still need to be added which are |
|
@bertdeblock let's land your changes first so I layer mine on top so I do not cause an conflicts |
aklkv
force-pushed
the
fix/align-linters-with-classic
branch
from
c8c533f to
fe5a944
Compare
|
Tyty |
|
Hmm. Node 20 is in maintenance mode and is not in active lts |
|
I think my change did not get merged into main yet #120 |
|
hmm -- lemme check the release.md |
|
you sure? |
|
yeah I mixing things up, i was jut working internally on upgrading node, my PR only bumped to v20, let me bump it to v22 |
aklkv
force-pushed
the
fix/align-linters-with-classic
branch
from
fe5a944 to
501bd01
Compare
files/package.json
Outdated
| }, | ||
| "engines": { | ||
| "node": ">= 20" | ||
| "node": ">= 22" |
There was a problem hiding this comment.
I am not sure what is policy around buming node version between ember-cli versions? @mansona
There was a problem hiding this comment.
we also need >= 22.16
There was a problem hiding this comment.
ok, new plan -- lets keep node @ 20!
I'll PR a fix for dirname
There was a problem hiding this comment.
(your PR didn't introduce the CI error, but revealed that we messed up elsewhere)
aklkv
force-pushed
the
fix/align-linters-with-classic
branch
from
501bd01 to
51fe88b
Compare
|
Some files need to be formatted again, because of the Prettier changes. For example: This file has missing trailing commas. EDIT: I see CI is already flagging this 👍 |
|
hi @aklkv any chance you could rebase? I think the underlying issues with CI and our node-testing have been worked out now |
aklkv
force-pushed
the
fix/align-linters-with-classic
branch
from
51fe88b to
6767015
Compare
aklkv
force-pushed
the
fix/align-linters-with-classic
branch
from
6d0f580 to
df6121e
Compare
NullVoxPopuli
left a comment
NullVoxPopuli
left a comment
There was a problem hiding this comment.
making some updates to reduce the diff
4 participants
There were some important changes in classic app blueprint which this new blueprint is missing