Skip to content
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

let there be let #2217

Merged
merged 4 commits into from
Apr 10, 2020
Merged

Conversation

Tobenna-KA
Copy link
Contributor

Hi, I have made changes to this file most of which is var to let. I also tested the changes to make sure nothing breaks. I had to leave some declarations with var because of their global use.

I also noticed something I think is a mistake on line 3289 of the current version and line 3272 of this commit. The variable selectedNote is declared and assigned a value but never used. I think this variable is meant to be selectednote. Because if you look in the current version, within the same scope on line 3306, there is a passing of an argument selectednote to function piemenuPitches() which I believe should be the value defined for selecteNote. If you could please look at it.

There were also place I moved the variable declaration to a more global location within the same scope though, to allow the use of let.

@walterbender
Copy link
Member

I think you are correct re selectednote vs selectedNote. I'l review tonight or tomorrow.

@walterbender walterbender merged commit 3419bb9 into sugarlabs:master Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants