Skip to content

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

3 participants