musicblocks icon indicating copy to clipboard operation
musicblocks copied to clipboard

Use arrow functions instead instead of normal ones

Open aviral243 opened this issue 4 years ago • 4 comments

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 bind this to use in surrounding code:

Screenshot from 2020-04-04 06-43-54

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.

aviral243 avatar Apr 04 '20 01:04 aviral243

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 avatar Apr 04 '20 02:04 quozl

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

aviral243 avatar Apr 04 '20 03:04 aviral243

Let's also have a task list to keep track:

  • [x] js/
  • [x] js/widgets/
  • [x] js/blocks/
  • [x] js/utils
  • [x] planet/

aviral243 avatar Apr 04 '20 03:04 aviral243

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.

walterbender avatar Apr 04 '20 11:04 walterbender

Still quite a few places we should make these changes.

walterbender avatar Jan 11 '23 02:01 walterbender

Hey , if this is still open . I want to contribute.

thapar-vansh avatar Jan 22 '23 13:01 thapar-vansh

Please make a pull request.

walterbender avatar Jan 22 '23 16:01 walterbender

It says I can't commit since I don't have access to musicblocks

thapar-vansh avatar Jan 22 '23 18:01 thapar-vansh

You cannot make a PR? Or you cannot merge your PR?

walterbender avatar Jan 22 '23 18:01 walterbender

Sounds like forgot to fork.

quozl avatar Jan 22 '23 20:01 quozl

I would love to contribute, can you assign it me?

guru9607 avatar Jan 29 '23 13:01 guru9607

We don't assign issues, but please feel free to make a pull requet.

walterbender avatar Jan 29 '23 13:01 walterbender

@walterbender can you review my pull request, I have added it just a moment ago. Let me know if there are any changes needed

RiteshSinha919 avatar Jan 29 '23 20:01 RiteshSinha919

Hey, Can I work on this issue?

simplysabir avatar Jan 30 '23 07:01 simplysabir

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

quozl avatar Jan 30 '23 09:01 quozl

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

RiteshSinha919 avatar Jan 31 '23 17:01 RiteshSinha919

I think he is going to reopen with just the changes to /js and not the other changes.

walterbender avatar Jan 31 '23 17:01 walterbender

@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

RiteshSinha919 avatar Jan 31 '23 17:01 RiteshSinha919

It would be much more useful if you'd work on one of the "good first" issues with more substance.

walterbender avatar Jan 31 '23 17:01 walterbender

Hey, I'm working at this issue right now, is that all right?

Vivekk-11 avatar Feb 03 '23 14:02 Vivekk-11

Looking forward to seeing the PR

walterbender avatar Feb 03 '23 15:02 walterbender

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 !!

Snach13 avatar Feb 04 '23 16:02 Snach13

  • [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

Snach13 avatar Feb 04 '23 18:02 Snach13

I see the issues aren't being assigned to anyone. Can I start working on it too?

Husain01 avatar Feb 06 '23 04:02 Husain01

Hey @walterbender, I've made a Pull Request. Could you see it, please?

Vivekk-11 avatar Feb 06 '23 04:02 Vivekk-11

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.

quozl avatar Feb 06 '23 07:02 quozl

Now should I just rebase from the other person's pull request branch?

Vivekk-11 avatar Feb 06 '23 09:02 Vivekk-11

I have solved this issue just look my Pull Request. Please

singhshivansh244 avatar Feb 06 '23 17:02 singhshivansh244

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.

quozl avatar Feb 06 '23 20:02 quozl

i am interested to work on this issue , can i start ? @quozl

hirentimbadiya avatar Feb 07 '23 10:02 hirentimbadiya