-
Notifications
You must be signed in to change notification settings - Fork 10
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
Vector map variant #19
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
=========================================
- Coverage 44.68% 42.5% -2.19%
=========================================
Files 5 6 +1
Lines 687 760 +73
Branches 133 140 +7
=========================================
+ Hits 307 323 +16
- Misses 380 437 +57
Continue to review full report at Codecov.
|
# Conflicts: # js/src/main/scala/scalajson/ast/JValue.scala # jvm/src/main/scala/scalajson/ast/JValue.scala
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.
I think we need some tests for VectorMap
itself.
Hopefully @SethTisue or someone else from the scala team can clarify, but is it appropriate to use the scala.collection
package here?
Maybe we should keep the VectorMap
class private for now and only use the Map
or immutable.Map
type in the public API.
@@ -0,0 +1,169 @@ | |||
package scala.collection.immutable |
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.
I would like to echo @gmethvin's concern here. I don't think we should use package scala
here.
/cc @SethTisue
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.
sorry I missed this before. while it's certainly possible this could eventually become part of the Scala 2.13 collections, for now it's not okay to use scala.collection
20c330b
to
1dc632d
Compare
1dc632d
to
c06e9e0
Compare
Change data structure of safe JObject from
Map
to a customVectorMap
implementation which preserves key ordering on insertion of elementsStill to do
VectorMap
(same asMap
implementation)