Method Fixes #1990
Method Fixes #1990
Conversation
github-actions
bot
added
the
pr: change existing extension
| ? Math.round(totalSeconds % 60) | ||
| .toString() | ||
| .padStart(2, "0") | ||
| ? Scratch.Cast.toString(Math.round(totalSeconds % 60)).padStart( |
There was a problem hiding this comment.
Changing toString() to Cast.toString() here has no value and if anything harms readability. You already know Math.round() is returning a number. Hope this isn't how the rest of the pull request looks.
There was a problem hiding this comment.
Conceptually Scratch.Cast would be where any quirks in the anything-to-string conversion in Scratch-the-programming-language would live though it happens to be exactly the same as the default toString(). It doesn't need to be the same though.
Here, and many other places, you aren't dealing with a value from a Scratch project but rather some intermediate value you already know is a number. It's not benefiting at all to use Cast. Just adding noise and overhead and making the code less portable if someone wants to take it outside of an extension.
| @@ -698,8 +700,8 @@ | |||
| } | |||
|
|
|||
| toCase(args, util) { | |||
| const string = args.STRING.toString(); | |||
| const textCase = args.TEXTCASE.toString(); | |||
| const string = Scratch.Cast.toString(args.STRING); | |||
There was a problem hiding this comment.
most of the changes in this extension do make sense though
|
Closed to be superseded by Miyo. |
Self explanatory.
I want a proper in depth review (Looking at you, SharkPool), so I can make sure I didn't miss anything or nothing is broken.