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

Add toIntOption (and Long, Boolean, et al) #566

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Conversation

rjolly
Copy link
Contributor

@rjolly rjolly commented Nov 3, 2022

fixes #554

@SethTisue SethTisue marked this pull request as draft November 3, 2022 17:11
@SethTisue
Copy link
Member

SethTisue commented Nov 3, 2022

looked at this with Raphael just now and some work items remain:

  • move the code to the scala_2.11_2.12 folder (hopefully a separate 2.11 version won't be needed)
  • refactor to remove the duplication of assertThrow*

@SethTisue
Copy link
Member

SethTisue commented Nov 3, 2022

perhaps @martijnhoekstra, the original author, would look to take a look, once this is in final form

@rjolly rjolly force-pushed the main branch 2 times, most recently from 915af6f to 3cb09f3 Compare November 8, 2022 08:36
@SethTisue
Copy link
Member

Looks like JS needs to expect a different exception:

[error] Test scala.collection.compat.StringParsersTest.nullDouble failed: Wrong exception: expected java.lang.NullPointerException but was scala.scalajs.js.JavaScriptException, took 0.002524921 sec
[58](https://github.com/scala/scala-collection-compat/actions/runs/3417699470/jobs/5689134821#step:5:59)

@SethTisue
Copy link
Member

as for Native,

[error] Test scala.collection.compat.StringParsersTest.floatSpecificTest failed: str.toFloat <> str.toFloatOption for NaNd, took 0.009 sec
[69](https://github.com/scala/scala-collection-compat/actions/runs/3417699470/jobs/5689134943#step:5:70)
[error] Test scala.collection.compat.StringParsersTest.doubleSpecificTest failed: str.toDouble <> str.toDoubleOption for NaNd, took 0.009 sec

@rjolly rjolly force-pushed the main branch 6 times, most recently from 944b869 to ead6848 Compare November 9, 2022 16:28
@rjolly
Copy link
Contributor Author

rjolly commented Nov 9, 2022

Seems like both JS and Native think that "NaNd" should parse, as opposed to JVM.

@rjolly rjolly marked this pull request as ready for review November 10, 2022 09:09
@SethTisue SethTisue self-assigned this Nov 10, 2022
@SethTisue SethTisue changed the title Add StringOps Add toIntOption (and Long, Boolean, et al) Nov 23, 2022
@SethTisue
Copy link
Member

SethTisue commented Nov 23, 2022

I added a commit that brings the readme up to date.

@scala/collections folks want to take a look before merge...?

@SethTisue SethTisue removed their assignment Nov 23, 2022
@SethTisue SethTisue mentioned this pull request Nov 23, 2022
@SethTisue SethTisue merged commit 0737983 into scala:main Nov 28, 2022
@SethTisue
Copy link
Member

thank you @rjolly!

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.

Add .toXOption methods from StringOps
3 participants