-
-
Notifications
You must be signed in to change notification settings - Fork 602
feat(core)!: remove rechoir + lodash templates from config loader #4066
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
Conversation
erikian
left a comment
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'm getting Error: Template (base) is not compatible with this version of Electron Forge (7.10.2), it requires 7.7.0 when running tests locally with git clean -dfx && yarn && yarn build && yarn test, maybe the build artifacts we're using on CI are stale?
| describe('forgeConfigIsValidFilePath', () => { | ||
| it('succeeds for a file extension-less path', async () => { | ||
| const fixturePath = path.resolve(__dirname, '../../fixture/dummy_js_conf/'); | ||
| await expect( |
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: this await is no longer necessary
|
|
||
| it('fails when a file is nonexistent', async () => { | ||
| const fixturePath = path.resolve(__dirname, '../../fixture/dummy_js_conf/'); | ||
| await expect( |
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: same here
MarshallOfSound
left a comment
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.
Approving the concept, have fun fighting CI
|
@erikian which tests in particular are failing locally for you? I'm not running into this locally myself with the same steps. 🤔 |
|
|
BTW, do we still need |
Those should be short-circuited via |
|
@erikian I wonder if you have Forge 7.7.0 installed globally by accident and that's polluting the tests. None of our fixtures seem to reference |
|
@erickzhao you're right, I do have |
rechoirandinterpretlodash.templatefor interpolated configs: https://lodash.com/docs/4.17.15#templateBREAKING CHANGE: Forge only accepts JS/TS configuration files