musicblocks
musicblocks copied to clipboard
let there be let
Now that ECMA is ubiquitous, there is no longer any reason to not use let instead of var where appropriate. It is a fairly major change -- we need to be careful regarding scoping. But nonetheless, it is time.
- [ ] js/
- [x] js/widgets/
- [x] js/blocks/ <-- mostly already in place
- [x] js/utils
- [x] planet/
I will be working on this issue.
I will be working on this issue.
Already started, this is actually in reply to the commit I made. Maybe we can talk about how to share the work, so we don't spend time working on the same things?
@Tobenna-KA ohh. same here. I also already started. Which part are you doing right now?
@Ishakikani9117 I was heading into js/
@Tobenna-KA great , I was doing blocks and i will be heading to widgets and utils.
Alright, sounds good, I will do the work on what is outside those 3 folders within /js
@walterbender Should we also try moving towards Arrow functions as there are numerous instances where this
binding has been an issue, in the entire code.
We can get rid of them using Arrow functions.
yes... we can use arrow functions in a lot of places.
@walterbender Will create a new issue and start working on it ASAP.
@Ishakikani9117 @Tobenna-KA no offense. But, please be careful with the variable scopes while replacing var
with let
. I'm sure you know that let
, unlike var
, has a block only scope.
Here is an example. #2214 is caused by such a mistake. See the following excerpt in FlowBlocks.js:
@Ishakikani9117 @Tobenna-KA no offense. But, please be careful with the variable scopes while replacing
var
withlet
. I'm sure you know thatlet
, unlikevar
, has a block only scope.Here is an example. #2214 is caused by such a mistake. See the following excerpt in FlowBlocks.js:
I'm pretty sure we both know this, and at least with the changes I have made in cases like this, you'd see me moving variable declarations out of the scope to a more global position to allow the use of let rather than var, but if necessary leave the var declaration.
But thank you for your concern.
@Tobenna-KA Are you working on the remaining files inside js/
folder? Once they're done I can resume working on #2201
@Tobenna-KA great , I was doing blocks and i will be heading to widgets and utils.
@Ishakikani9117 I was recently working on quite a few issues with the phrase maker (widgets/pitchtimematrix.js). There's one I'm currently working on and it might require changes at several places. So, can you please leave out that file for the time being? After the issue is sorted, I can help with the replacements by let
and arrow
functions for that file, since I'm somewhat versed with its contents.
@meganindya Ok. I will leave that file.
@Tobenna-KA Are you working on the remaining files inside
js/
folder? Once they're done I can resume working on #2201
Yes I am
@meganindya @Ishakikani9117 @Tobanna-KA Is anyone doing changes to planet. I want to work on this if anyone has not yet started on this.
@b18050 I want to do the planet folder.
@walterbender I've reviewed pull requests for changes in planet/
(#2231) and js/utils
(#2225, #2226). Please take a look.
The bulk of the work remaining is with the widgets.
While I marked this as a "Good first issue", please bear in mind: (1) global replace will undoubtedly break the code so please don't; (2) please test after making your changes.
While I marked this as a "Good first issue", please bear in mind: (1) global replace will undoubtedly break the code so please don't; (2) please test after making your changes.
@walterbender remaining files of js/widgets
have been cleaned up, and both the guidelines were kept in mind while doing so.
can i work on this issue ?
The js/
directory is the priority. Only 5 effective occurrences remain, probably corner cases. You may try #2629.
ohk thanks
Please allow me to further clean it up in the remaining files.
@walterbender @meganindya is there any work still pending?