mavo
mavo copied to clipboard
mavoweb
[Chore] Use let instead of var throughout the codebase
Also, should we add an ESLint rule forbidding var? I guess we'll have a better idea after the refactor (we can see if there are valid use cases for which function scope makes more sense)
This is actually not a good first issue, as it requires very extensive changes to the codebase. Large PRs, especially from first time contributors, take a lot of resources to review and I'm not comfortable merging large PRs from first-time contributors without substantial review.
I think we should take a gradual approach: every time we touch a piece of code, we convert its variables to use let. Eventually, over time, we'll have very few vars left, and we can just address them with a find-and-replace that is actually reviewable.
@moralcode Does that make sense?
Case in point: I just pushed https://github.com/mavoweb/mavo/commit/d76f4c6c2c10953edb3b784c829d72f9952056d3 which I thought was small enough, so it should be ok. But alas, it turned out I had introduced a syntax error (fixed by https://github.com/mavoweb/mavo/commit/60b25db39fa8f1d03f9ad7512270d2f0ccb0b0a6 )!
@MoralCode Does that make sense?
Yep! i think thats mainly what i was starting to go for when i re-did (aka force-pushed) my pull request to remove a lot of the find-and-replace junk and just keep the manual changes I made to fix some simple places where variables needed to be moved up in scope (basically more of what you did in 60b25db).
i realize my PR is a little shotgun approach-y compared to your one-method-at-a-time push though. I mostly just figured it would be helpful to make the most use of what few changes I did make, but feel free to do whatever you want with it
is there any refactor to do ?
No, we edit these when we touch the corresponding part of the code, and we'll close this issue when we've converted the whole codebase (we can add a commit to fix the few remaining ones, when few remain, but right now it would be a very extensive commit that would mess up git history and introduce potential for regressions).