Skip to content
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 declarations instead of var wherever constants are declared #614

Closed
Jaifroid opened this issue Apr 14, 2020 · 8 comments
Closed

Comments

@Jaifroid
Copy link
Member

We have established with #613 that const works with all the browsers we support, and it improves readability as well as (presumably) improving browser/app memory management (likely a very tiny effect).

@Jaifroid Jaifroid added this to the v2.9 milestone Apr 14, 2020
@Jaifroid Jaifroid self-assigned this Apr 14, 2020
@yonis9
Copy link

yonis9 commented Apr 14, 2020

Hi, can I take it?

@Jaifroid
Copy link
Member Author

@yonis9 Yes, this would be an easy first PR. Just make sure that it is only constants that you define with const, i.e. nothing that can change values. Generally we indicate constants with capital letters. For example, you'll see three at the beginning of app.js.

@ritikkuril
Copy link

can i take this issue

@Jaifroid
Copy link
Member Author

Yes you can, thanks.

@ritikkuril
Copy link

i new to this so just asking i just need to convert all var type in const type in all .js scripts

@Jaifroid
Copy link
Member Author

@ritikkuril No, as is written above, only values that are actually constants should be converted.

@fabcoder547
Copy link

@ritikkuril have you done setup on your localpc? can you guide me in that ? how you did that?

@ritikkuril
Copy link

@fabcoder547 i am also new bro

@kelson42 kelson42 linked a pull request Jun 25, 2020 that will close this issue
@Jaifroid Jaifroid modified the milestones: v3.3, v3.1 Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants