-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Use const
for requires
#4109
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
Use const
for requires
#4109
Conversation
|
||
var slice = [].slice; | ||
|
||
var appPath; |
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 think this was the first file we worked on, do we want to inline these now like we ended up doing in the other files?
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.
oh yeah, I'll do that 👍
for (i = 0, len = ref1.length; i < len; i++) { | ||
name = ref1[i]; | ||
var name = ref1[i]; |
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.
The var deceleration should be outside of the loop (line 63).
@@ -164,8 +162,9 @@ module.exports = { | |||
}; | |||
|
|||
// Mark standard asynchronous functions. | |||
ref1 = ['showMessageBox', 'showOpenDialog', 'showSaveDialog']; | |||
var ref1 = ['showMessageBox', 'showOpenDialog', 'showSaveDialog']; | |||
var j, len |
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.
Super minor, but missing a ;
here
👍 |
We also inlined a few vars that coffee had listed at the top of each file.
A part of #4065