-
Notifications
You must be signed in to change notification settings - Fork 811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port remaining files completely to ES6+ syntax and modules #2629
Comments
This need not be done all at once, rather it must be done incrementally lest the changes should be too invasive. I'll start working with |
@meganindya I would like to help with this too. I can start with porting to class definition. |
Pls go ahead. Maybe start with the smaller ones, e.g. widgets. Be careful though. Add me as a reviewer on the PR (better one file at a time). |
@meganindya sounds great!
I am starting off by porting var to let/const. Thankyou for the articulate explanation of what needs to be done. |
One important advice: NEVER DO A REPLACE ALL WHEN THERE ARE A LOT OF CASES. We have had trouble before. Hopefully, you're aware about the scoping issue with |
I have started porting |
I would I like to help with this,I will change callbacks to arrow functions. @meganindya |
@meganindya i think there is only one /blocks.js that has not been covered yet? |
I'm working on blocks.js. |
@dhruv-doshi cool ! |
@endurance21 perhaps you can try your hand at #2632. |
Thanks @meganindya |
Hey @meganindya I was looking at #2637 and the changes seem to be similar to what I have been editing and the old PR was merged. I would like to know what was the change that has to be made. Thanks |
I can't seem to figure where those changes went. The code is still the old one. Weird! Anyway, if you have a more tried and tested version of the ES6 port, please create a PR. Otherwise, I could copy the changes from #2637. |
I would love to contribute the code. It shall be a learning experience for me. |
Go ahead. |
I would love to contribute by converting the functions to ES6 format. |
Hey @meganindya |
hello @meganindya And sir as you mention about MB.2 , will it be have a new structure of code or addon over existing architect of musicblocks. Any of the framework is also be taken into consideration like React? |
I would like to work on the remaining files @meganindya. Should I start working on it? |
Hi @meganindya I also wanted to a helping hand in this can you tell me where to begin |
Hi, @meganindya please assign me a function which is available. I am new to contributing in sugarlabs |
Hey, @meganindya I'm a newbie and would love to contribute. |
Music Blocks is being built from the ground-up, to address several architectural problems with this run. Since Music Blocks is a fork of Turtle Blocks JS, musical functionality was added on top of it. However, music is fundamental to Music Blocks. Besides, the Turtle Blocks JS started initially with handful of features and was written without a complex architecture. As Music Blocks got built on top of that, it got incrementally complex, but the architecture remained simple, thus resulting in a monolith. Also, the functionality is tightly coupled with the interface and native client API (Web API). Keeping these problems in mind, we have considered a foundational rebuild that will address all these issues, whilst adding buffers for future additions. We'll also be using a more elegant tech-stack to develop and maintain this project given its scale. Refer to the repository sugarlabs/musicblocks-v4 for more information about the new project — Music Blocks (v4). |
If this is open, I can work on this |
Hi, The /js/blocks.js file doesn't have the class definition, can I work on it? |
Please see #2629 (comment) |
There are some files which I have ported today only to ES6+ syntax and modules such as js/block.js and other files. Are they ported already or they are left? If they are not ported, shall I send a PR? @meganindya |
Most of the JavaScript files contain code a lot of ES5 code, that better be updated to ES6 (and above, e.g.
async
/await
). It's not like we should be bothered about ceding any further browser support because the codebase already contains ES8 elements.As for the ES5-ES6 syntax, we should port to the following things:
var
tolet
/const
arrow
functions in callbacksprototype
definition toclass
definition (consider use ofstatic
members too)for ... in
/for ... of
loops wherever appropriateIn addition, replace function declaration within functions with
const
function variable declarations. Consider effects of hoisting.Replace
with
Port to
class
definition and more:block.js
blockfactory.js
blocks.js
boundary.js
languagebox.js
palette.js
pastebox.js
pluginsviewer.js
protoblocks.js
saveInterface.js
toolbar.js
trash.js
widgets/help.js
widgets/meterwidget.js
widgets/modewidget.js
widgets/musickeyboard.js
widgets/oscilliscope.js
widgets/phrasemaker.js
(rename class fromPitchTimeMatrix
toPhraseMaker
)widgets/pitchdrummatrix.js
widgets/pitchslider.js
widgets/pitchstaircase.js
widgets/rhythmruler.js
widgets/statistics.js
widgets/status.js
widgets/temperament.js
widgets/tempo.js
widgets/timbre.js
widgets/widgetWindows.js
Even though the
js/blocks/
API uses classes. Most of the code needs cleanup/refactoring. A lot of the classes aren't even used (their corresponding blocks have been deprecated). Proper identification and documentation can possibly be helpful when we port the blocks. Do not remove any code though. Unused variables can be gotten rid of.The text was updated successfully, but these errors were encountered: