-
-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Drop support for Ember < 4.8 and node < 18 #1655
Conversation
eac1690
to
9313ff6
Compare
runs-on: ${{ matrix.os }} | ||
strategy: | ||
matrix: | ||
os: [macos-latest, ubuntu-latest, windows-latest] | ||
package-manager: ['npm', 'yarn'] |
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.
Do we not want to check multiple package managers anymore?
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.
I don't really think we need to. Since we don't run the blueprint automatically, I think the actual package install is just a regular package install. So I can't think of anything we'd be testing by testing multiple package managers other than the package managers themselves? Maybe I'm missing/forgetting something?
583e937
to
a18fef7
Compare
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.
LGTM! I think we need a rebase though.
Analytics were removed from ember-cli, so we don't need to mock them (and can't because the `mock-analytics` module was also deleted! Note that we're leaving the `this.analytics` references in `electron.js` to maintain compatibility with `ember-cli` < 5.4
There's no longer a reason to test against both npm and yarn, so simplify!
Enable the `--test-port` option. It's a good one to have regardless, but also logic inside `ember-cli`'s `test` command blows up if the option isn't present.
This used to be an override of a method in the base class, but that method is long gone...
a18fef7
to
bc98d8e
Compare
Bumps minimum Ember to 4.8 and minimum node to 18