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

Various additions #1

Merged
merged 11 commits into from
Aug 26, 2020
Merged

Various additions #1

merged 11 commits into from
Aug 26, 2020

Conversation

jodersky
Copy link

@jodersky jodersky commented Aug 6, 2020

Currently WIP, these changes aim to bring the upickle community build up to feature parity with scala 2.

Some notable changes:

  • add macroR, macroW and macroRW
  • allow overriding field names through the upickle.implicits.key annotation
  • allow unknown keys in reader
  • handle missing keys in reader

Many tests still fail and/or need to be reactivated

Jakob Odersky added 8 commits August 6, 2020 12:32
Instead of defining derivation via a given, make it explicit via `macroR`
and `macroW` defs. This also copies upickle's original behaviour.

Also remove wildcard imports from the compiletime and deriving packages.
Comment on lines +6 to +12
def serializeDefaults: Boolean = false

def objectAttributeKeyReadMap(s: CharSequence): CharSequence = s
def objectAttributeKeyWriteMap(s: CharSequence): CharSequence = s

def objectTypeKeyReadMap(s: CharSequence): CharSequence = s
def objectTypeKeyWriteMap(s: CharSequence): CharSequence = s
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this stuff was moved from Api

Copy link

@anatoliykmetyuk anatoliykmetyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM. Are the Dotty and Scala 2 tests passing locally?

end getDefaultParmasImpl

inline def summonAll[T <: Tuple]: List[_] =
inline def summonList[T <: Tuple]: List[_] =

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

summonAll was defined here temporarily while we didn't have one in Dotty. Now we do. Can we completely remove summonAll and summonList from here and use the Dotty implementation?

@jodersky jodersky merged commit f91cacf into dotty-community-build Aug 26, 2020
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.

2 participants