Skip to content

refactor var usage to const/let #429

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

Merged
merged 2 commits into from
Feb 20, 2017

Conversation

nicknisi
Copy link
Collaborator

Remove all var usage in the project and tests, and replace it with const/let usage. This PR touches a lot of files but the only changes in each file are to convert varto const or let, and to move the declaration of variables based on the new scope rules when necessary. This is part of the refactor updates mentioned in #378.

Remove all var usage in the project and tests, and replace it with const/let usage.
@nicknisi
Copy link
Collaborator Author

I updated the commit and reverted the changes to the tests. I forgot that let and const don't work in all cases in Node 4. :-)

@nicknisi
Copy link
Collaborator Author

Also, it might be worth looking into converting the tests to TS in another PR.

bin/typedoc Outdated
@@ -1,4 +1,4 @@
#!/usr/bin/env node

var td = require('../dist/lib/cli.js');
const td = require('../dist/lib/cli.js');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we leave this one as var since it's JavaScript?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and put the var back here. It should be fine, as that usage of const is ok in Node 4, but just to be consistent, it seemed better to put it back.

@blakeembrey blakeembrey merged commit 026cb81 into TypeStrong:master Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants