-
Notifications
You must be signed in to change notification settings - Fork 822
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
js/turtleactions: refactor, lint & prettify #2813
Conversation
@meganindya pls review this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be more helpful if we had where the globals come from in the same file. I think the reason behind enabling linting was that we could identify and fix other issues in the process. I guess, fixing linting issues in a way that just make the errors go away is as good as not having linting at all. Just my humble opinion. Thanks
Totally agree. But that's not what I did. Core of MusicBlocks resides in the Thanks a lot for the review btw |
@meganindya Please go through my comments in my review which have been marked as resolved. I was working on some other issue that involved one of the files in this PR so I thought reviewing this would be a good opportunity to learn. Thanks |
@ksraj123 Thx for pointing out your concerns. I have unresolved your comments on the review, waiting for better answers to these. Surely it's a great opportunity to learn more about how the codebase is inter-connected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the locations too
/*
Global locations
path..
.., .., ...
*/
Thanks for pointing out my mistakes and sharing your feedback @meganindya |
|
@meganindya Pls review and provide feedback, thanks. |
issue references: #2609, #2767