-
Notifications
You must be signed in to change notification settings - Fork 154
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
Fixed #707 Replaced var with const and let and added strict mode #712
Conversation
Codecov Report
@@ Coverage Diff @@
## master #712 +/- ##
=======================================
Coverage 86.71% 86.71%
=======================================
Files 16 16
Lines 1746 1746
=======================================
Hits 1514 1514
Misses 232 232
Continue to review full report at Codecov.
|
Hi hjpatel16, great edit of your file, however you forgot to change lines 14-16 from var to let/const. Also, though it may be redundant, please include a description of what you did to the code. |
Thanks @DavidLi119, I was confused to use let for those remaining variables but I did make some changes |
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.
This is looking good. I left a few notes of places where you should use const
vs. let
. Also, I notice that your Pull Request doesn't have a description. Make sure you always include one, and also reference the issue number you are fixing, for example, "Fixes #707".
Let me know when you've made these changes and pushed again, and I'll re-review.
src/fs-watcher.js
Outdated
@@ -46,12 +48,12 @@ function FSWatcher() { | |||
recursivePathPrefix = filename === '/' ? '/' : filename + '/'; | |||
} | |||
|
|||
var intercom = Intercom.getInstance(); | |||
let intercom = Intercom.getInstance(); |
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.
This could be const
, since intercom
won't change again.
src/fs-watcher.js
Outdated
intercom.on('change', onchange); | ||
}; | ||
|
||
self.close = function() { | ||
var intercom = Intercom.getInstance(); | ||
let intercom = Intercom.getInstance(); |
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.
Same thing here, you can use const
Fixed #707 for fs-watcher.js file to use let and const instead of var where applicable. You can check under the files changed tab to review my changes |
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.
Great, looks good.
No description provided.