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

Modern JS: don't use var in css-examples #163

Open
teoli2003 opened this issue Dec 7, 2023 · 15 comments
Open

Modern JS: don't use var in css-examples #163

teoli2003 opened this issue Dec 7, 2023 · 15 comments
Labels
chore A routine task. effort: medium Task is a medium effort. good first issue A good issue for newcomers to get started with. help wanted If you know something about this, we would love your help! idle Issues and pull requests with no activity for three months.

Comments

@teoli2003
Copy link
Contributor

Quite a few examples in this repo extensively use var.

This is a bad practice; we have eliminated them from mdn/content (and other repos). We should do the same here: most will be simply replaced by const and let, although there may be a few more complex cases (where var is used inside a scope for a variable at a higher scope)

@teoli2003 teoli2003 added help wanted If you know something about this, we would love your help! good first issue A good issue for newcomers to get started with. chore A routine task. effort: medium Task is a medium effort. labels Dec 7, 2023
@pragyamishra56
Copy link

@teoli2003 Sir If it possible to allow me to fix this issue ?

@teoli2003
Copy link
Contributor Author

Just go ahead; it is yours. Don't create too big PRs, they take longer to review.

@pragyamishra56
Copy link

@teoli2003 Sir Thanks for pointing this out! I completely agree with eliminating the use of 'var'. It's considered a best practice to use 'const' and 'let' for better scope management. I'll work on addressing this across the repository, replacing 'var' with the appropriate declarations. Your input is highly appreciated!

@pragyamishra56
Copy link

@teoli2003 Sir, Replace extensive var usage with const/let, aligning with the best So, should I replace 'var' with 'const/let'?

@teoli2003
Copy link
Contributor Author

Yes.

@pragyamishra56
Copy link

@teoli2003 Sir, Could you confirm that In every JS file, I will replace the var with const/let?

@teoli2003
Copy link
Contributor Author

With the relevant one, yes. If you are not sure, start with one file.

@pragyamishra56
Copy link

@teoli2003 Sir, Issue #163 is resolved now could you please have a look at it if any issue is raised then let me know

@teoli2003
Copy link
Contributor Author

You need to open a pull request.

@pragyamishra56
Copy link

@teoli2003 Sir I opened a pull request could you review it if any further issue is raised then let me know

@pragyamishra56
Copy link

@teoli2003 Sir, could you respond to my PR? If any further issues are raised then let me know

@hamishwillee
Copy link
Collaborator

@teoli2003 Responded here: #171 (comment) - essentially you can't just search and replace across the examples to fix this. Because each case needs a separate check and review, it would be better if this was split across a number of PRs

@pragyamishra56
Copy link

@hamishwillee Sir I appreciate the clarification. I agree that a more thorough approach is needed. I'll work on breaking down the changes into separate PRs for a more comprehensive review. Thanks for guiding me through this process.

@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Mar 7, 2024
@Mosespimor
Copy link

Quite a few examples in this repo extensively use var.

This is a bad practice; we have eliminated them from mdn/content (and other repos). We should do the same here: most will be simply replaced by const and let, although there may be a few more complex cases (where var is used inside a scope for a variable at a higher scope)

@github-actions github-actions bot removed the idle Issues and pull requests with no activity for three months. label Jun 8, 2024
@pragyamishra56
Copy link

@teoli2003 Sir, if you agree, can I make changes to each file individually and then create pull requests? You mentioned that I should start with one file, it will be reviewed, and the pull request will be merged.

@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A routine task. effort: medium Task is a medium effort. good first issue A good issue for newcomers to get started with. help wanted If you know something about this, we would love your help! idle Issues and pull requests with no activity for three months.
Projects
None yet
Development

No branches or pull requests

4 participants