Fix electron build #1177
Fix electron build #1177
Conversation
| "min_width": 400, | ||
| "min_height": 200 | ||
| } | ||
| "main": "electron/main.js" |
There was a problem hiding this comment.
I'm not sure about this line, I think it may point npm commands to electron folder and may result difficulty in debugging as execution will not be in the public
I will need to read up more and test this
There was a problem hiding this comment.
Good catch 👍 this point is tricky. Electron Application Architecture guide (https://electronjs.org/docs/tutorial/application-architecture) says that electron runs the script present at main inside the package.json
I see 2 possibilities :
- Keep the configuration as it is with an electron specific file hidden in
public/main.js(previous configuration) - Make an electron folder to name things and mode
public/main.jshere to clearly states it is for electron (what I chose)
The rest of ungit always run the ./bin/ungit binary and is not affected by this change
There was a problem hiding this comment.
I haven't done the packaging node projects and I'm not too certain on this matter but I think previous option makes more sense as how we package our app should not affect how the app should be built.
| @@ -496,7 +471,7 @@ module.exports = (grunt) => { | |||
| grunt.registerTask('publishminor', ['default', 'test', 'release:minor']); | |||
|
|
|||
| // Create electron package | |||
| grunt.registerTask('package', ['clean:electron', 'clean:babel', 'babel:electron', 'copy:electron', 'electron']); | |||
| grunt.registerTask('package', ['clean:electron', 'clean:babel', 'babel:electron', 'electron']); | |||
There was a problem hiding this comment.
We maybe conflating two PRs but we are doing babel for electron and removing babel in #1178.
We want to be consistent either keep babel and keep wider support or just remove it.
|
Superseded by #1240 |
Related to #1173
Electron changed some apis https://electronjs.org/blog/electron-api-changes