musicblocks
musicblocks copied to clipboard
Use arrow functions instead instead of normal ones
As we move towards using let
instead of var
, we can also make use of another ES6 Javascript feature; Arrow functions. The benefits of using it will be:
- Major benefit would be getting rid of
this
binding. e.g We come across instances of such format in our code where we need to bindthis
to use in surrounding code:
Arrow functions do that automatically.
- Allows implicit return in case there is no body of a function.
e.g. An arrow function with no parameters and no body
let func = () => 1;
- Syntax is cleaner than the old one. e.g. Functions with one parameter can also be defined without parantheses
let func = x => { return 2*x; }
Additional Context: I am working on this.
I approve. ES6 was finalised in June 2015. This along with the other changes relating to ES6 moves Music Blocks beyond compatibility with most instances of Sugar. I'm fine with that, as it was never released for older platforms. I'll make sure to advise against trying it.
@quozl Thanks for approving the change. I don't know much about the compatibility at this instant to comment on that, but it's good that new issues aren't popping up.
Let's also have a task list to keep track:
- [x] js/
- [x] js/widgets/
- [x] js/blocks/
- [x] js/utils
- [x] planet/
Re James' comment, I had been pretty diligent about keeping E6 out of MB for a long time, but during GCI, I had let down my guard, so that bridge has already been crossed.
Still quite a few places we should make these changes.
Hey , if this is still open . I want to contribute.
Please make a pull request.
It says I can't commit since I don't have access to musicblocks
You cannot make a PR? Or you cannot merge your PR?
Sounds like forgot to fork.
I would love to contribute, can you assign it me?
We don't assign issues, but please feel free to make a pull requet.
@walterbender can you review my pull request, I have added it just a moment ago. Let me know if there are any changes needed
Hey, Can I work on this issue?
@simplysabir we don't assign issues, but please feel free to make a pull request. You should have seen that said already in this issue. If you have seen somewhere that says you have to ask before fixing a problem, please point it out to us so we can correct it!
We like to merge the best possible pull request.
@walterbender is there any need to change the /js as mentioned by @aviral243. He already did these changes but didn't merge it, rather he closed the pull request.
I think he is going to reopen with just the changes to /js and not the other changes.
@walterbender Can I do this for a time being ? I think /js is the last file to be updated. We can close this issue with that being done
It would be much more useful if you'd work on one of the "good first" issues with more substance.
Hey, I'm working at this issue right now, is that all right?
Looking forward to seeing the PR
I'm converting normal functions into arrow functions
- [x] (js/abc.js)
- [] (js/activity.js) - this file is too big so I don't think all the functions converted to the arrow
- [x] (js/basicblocks.js)
and working forward !!
- [x] (js/block.js ) file checked
- [x] (js/lilypond.js) file checked
- [x] (js/macros.js) file checked
- [x] (js/mxml.js) file checked
- [x] (js/pallete.js) file checked
- [x] (js/piemenus.js) file checked
I see the issues aren't being assigned to anyone. Can I start working on it too?
Hey @walterbender, I've made a Pull Request. Could you see it, please?
We support any number of pull requests. When working closely with others, and the changes are likely to be merged (as in this case because they are straightforward), it can help if you rebase from each other's pull request branches. That means the maintainer may need to only merge one pull request instead of several.
If there's something about the other person's commits that mean you won't rebase or cherry-pick them, then that means you really ought to talk to them about it in their pull request.
Now should I just rebase from the other person's pull request branch?
I have solved this issue just look my Pull Request. Please
Thanks. It's great that so many people are helping. If you are waiting for review, please take a moment to review one of the other pull requests on the same issue.
The recent pull requests relating to this issue are;
- https://github.com/sugarlabs/musicblocks/pull/3158
- https://github.com/sugarlabs/musicblocks/pull/3160
- https://github.com/sugarlabs/musicblocks/pull/3161
- https://github.com/sugarlabs/musicblocks/pull/3162
- https://github.com/sugarlabs/musicblocks/pull/3155
You might detect mistakes, accidental changes, help maintain consistency and code quality.
You might test the changes to see if they work for you; if so, post a comment to say what you tested. Be sure to test MusicBlocks first without the changes, we don't want to hear about your problems testing that aren't related to the pull requests.
See our Guide for Reviewers.
i am interested to work on this issue , can i start ? @quozl