-
Notifications
You must be signed in to change notification settings - Fork 190
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
Scala 2.13.0-M4 #263
Scala 2.13.0-M4 #263
Conversation
trait ProductFormatsInstances { self: ProductFormats with StandardFormats => | ||
[# // Case classes with 1 parameters | ||
|
||
def jsonFormat1[[#P1 :JF#], T <: Product :ClassManifest](construct: ([#P1#]) => T): RootJsonFormat[T] = { | ||
val Array([#p1#]) = extractFieldNames(classManifest[T]) | ||
val Array([#p1#]) = extractFieldNames(implicitly[ClassManifest[T]]) |
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.
@@ -29,7 +31,7 @@ trait ProductFormatsInstances { self: ProductFormats with StandardFormats => | |||
fields.sizeHint(1 * 2) | |||
[#fields ++= productElement##2Field[P1](fieldName1, p, 0)# | |||
] | |||
JsObject(fields: _*) | |||
JsObject(fields.toSeq: _*) |
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.
varargs parameter type is collection.immutble.Seq
instead of collection.Seq
since Scala 2.13.0-M4
@@ -25,7 +27,7 @@ trait CollectionFormats { | |||
implicit def listFormat[T :JsonFormat] = new RootJsonFormat[List[T]] { | |||
def write(list: List[T]) = JsArray(list.map(_.toJson).toVector) | |||
def read(value: JsValue): List[T] = value match { | |||
case JsArray(elements) => elements.map(_.convertTo[T])(collection.breakOut) | |||
case JsArray(elements) => elements.map(_.convertTo[T]).toList |
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.
collection.breakOut
removed since Scala 2.13.0-M4
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.
This might have performance implications. breakOut
prevented intermediate collections from being created.
_.asInstanceOf[JsObject].fields("questions").asInstanceOf[JsArray].elements.size | ||
} === List.fill(20)(100) | ||
val list = Await.result( | ||
Future.traverse(List.fill(20)(largeJsonSource))(src => Future(JsonParser(src))), |
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.
use Future
instead of parallel-collections because parallel-collections for Scala 2.13.0-M4 does not available yet 😕
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.
Good, we're not in love with the par collections anyway :)
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.
More painless than I'd have expected, nice!
Perhaps we could avoid tests that depend on internal implementations of scala.Map entirely?
val v = scala.util.Properties.versionNumberString | ||
// TODO this test depeneds on internal implementations of scala.Map | ||
// https://github.com/scala/scala/commit/6a570b6f1f59222cae4f | ||
if (v != "2.13.0-M4") { |
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.
That's pretty nasty... perhaps we could test this by parsing the formatted JSON string and checking it's equivalent to the original?
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.
+1
val v = scala.util.Properties.versionNumberString | ||
val value = TestMangled(42, "Karl", true, 26, 1.0f).toJson.compactPrint | ||
// TODO this test depeneds on internal implementations of scala.Map | ||
if (v.matches("2\\.1[012].*")) { |
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.
Same here?
can this move forward now...? |
I'm going to finish it and release an M4 version so we can unblock downstream projects. |
Thanks a lot, @xuwei-k! |
No description provided.