-
Notifications
You must be signed in to change notification settings - Fork 155
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
Fixes Issue#674: Refactored var declarations to const or let declarations to address i… #676
Conversation
Codecov Report
@@ Coverage Diff @@
## master #676 +/- ##
=======================================
Coverage 86.71% 86.71%
=======================================
Files 16 16
Lines 1746 1746
=======================================
Hits 1514 1514
Misses 232 232 Continue to review full report at Codecov.
|
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.
Good initiative ndngo, I am wondering if it would be best to make more use of const in the file however. I notice a lot of use of let when the function is not actually changing any values. It might be best in such cases to use const instead.
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 'let' will be fine as there are multiple declarations of variables with the same name. By using let as the data type for "fs" and "fsPromise" will make them block-scoped. Check out this link.
Agree that |
That makes sense. I have changed variables that don't change values to be declared with const. |
Thanks for the clarification, I have also changed the code to use |
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.
Looks great.
…issue#674
Added the 'use strict' declaration at the top and refactored var variable declarations to use const or let.