search

mdn / css-examples

Code examples that accompany the MDN CSS documentation
https://developer.mozilla.org/docs/Web/CSS
Creative Commons Zero v1.0 Universal
618 stars 854 forks source link

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

Open teoli2003 opened 11 months ago

teoli2003 commented 11 months ago

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)

pragyamishra56 commented 10 months ago

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

teoli2003 commented 10 months ago

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

pragyamishra56 commented 10 months ago

@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 commented 10 months ago

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

teoli2003 commented 10 months ago

Yes.

pragyamishra56 commented 10 months ago

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

teoli2003 commented 10 months ago

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

pragyamishra56 commented 10 months ago

@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 commented 10 months ago

You need to open a pull request.

pragyamishra56 commented 10 months ago

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

pragyamishra56 commented 10 months ago

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

hamishwillee commented 10 months ago

@teoli2003 Responded here: https://github.com/mdn/css-examples/pull/171#issuecomment-1907681148 - 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 commented 10 months ago

@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.

Mosespimor commented 5 months ago

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)

pragyamishra56 commented 5 months ago

@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.