chore: replace all vars with const
#5875
Conversation
A quick find and replace requested by @benjamingr
|
in the past we have opted to not do project wide churn like this. Personally -1. |
|
Sorry, but we won't be doing this. This would create conflicts on literally every backport. |
|
I'd rather have several small PRs so it would be easier to review. I asked for small changes, 58 files is really hard to review and as @thealphanerd stated it's something that's not typically done at Node. |
|
Agree with the others, this needs to be done in smaller chunks. |
| var self = this; | ||
| var handled = false; | ||
| const self = this; | ||
| const handled = false; |
There was a problem hiding this comment.
This value is possibly modified, const will throw an error.
There was a problem hiding this comment.
This is one of the reasons smaller PRs are preferable.
|
Sorry. Going to close this. |
|
@SomeKittens fwiw thank you for making the PR! If you are looking for other ways to contribute please come hangout in #node-dev and we can find other places where you can help out! |
|
Yup, sorry, misunderstanding between @benjamingr and I. Carry on! |
Pull Request check-list
Please make sure to review and check all of these items:
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
test (or a benchmark) included?
existing APIs, or introduces new ones)?
Description of change
A quick find and replace requested by @benjamingr
Tests were not performed, will be CI'd as mentioned here