Skip to content

Add Number.parseInt + Number.parseFloat#1491

Merged
Perryvw merged 2 commits intoTypeScriptToLua:masterfrom
Z3rio:number-parseint
Sep 24, 2023
Merged

Add Number.parseInt + Number.parseFloat#1491
Perryvw merged 2 commits intoTypeScriptToLua:masterfrom
Z3rio:number-parseint

Conversation

@Z3rio
Copy link
Copy Markdown
Contributor

@Z3rio Z3rio commented Sep 24, 2023

Resolves issue: #1471

This maps Number.parseInt to parseInt and Number.parseFloat to parseFloat

@Zamiell
Copy link
Copy Markdown
Contributor

Zamiell commented Sep 24, 2023

Thank you!

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Sep 24, 2023

@Z3rio
Copy link
Copy Markdown
Contributor Author

Z3rio commented Sep 24, 2023

Sure ^^

@Z3rio
Copy link
Copy Markdown
Contributor Author

Z3rio commented Sep 24, 2023

Would it perhaps look something like that?
Considering parseInt & parseFloat already exist, and they do the same thing as Number.parseInt & Number.parseFloat, I based it off of those test cases.

I haven't really worked a lot with Jest though, so would be great if someone could look over it :)

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Sep 24, 2023

Maybe a little extreme rerunning all the tests since like you said the functions already existed and were tested throroughly, but from tests-as-specification point of view I guess this is alright

@Z3rio
Copy link
Copy Markdown
Contributor Author

Z3rio commented Sep 24, 2023

Maybe a little extreme rerunning all the tests since like you said the functions already existed and were tested throroughly, but from tests-as-specification point of view I guess this is alright

Yeah, it should probably be possible to modify the old tests to test it both without and with the Number. prefix.
Would that be preferable? If so, I'll go ahead and take a look at that.

@Perryvw Perryvw merged commit 79f8d1b into TypeScriptToLua:master Sep 24, 2023
@Z3rio
Copy link
Copy Markdown
Contributor Author

Z3rio commented Sep 24, 2023

<3

@Perryvw Perryvw mentioned this pull request Oct 1, 2023
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.

3 participants