-
Notifications
You must be signed in to change notification settings - Fork 279
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
Lift-mapper with Scala 2.13 support #1982
Lift-mapper with Scala 2.13 support #1982
Conversation
lvitaly
commented
May 31, 2020
•
edited
Loading
edited
- Lift-mapper was updated for supporting Scala 2.13
- All deprecation warnings of compiler were fixed
- Scala versions were bumped to 2.12.11 and 2.13.2
- Sbt version was bumped to 1.3.12
- scala versions were bumped to 2.12.11 and 2.13.2 - sbt version was bumped to 1.3.12
- specs2 was updated to 4.9.4 - scalatest was updated to 3.1.2 - apacheds dependency was removed as not used
case Some(join) => | ||
join | ||
} | ||
}(join => join) |
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.
What tests is this block covered under?
this.set(v) | ||
} | ||
|
||
def apply[Q <% FieldType](v: Q): OwnerType = { | ||
def apply[Q](v: Q)(implicit implFn: Q => FieldType): OwnerType = { |
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.
Why is the implicit required in this new version?
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.
Q <% FieldType
is a view bound syntax which is deprecated in Scala 2.13.x
- View bounds deprecated
- Deprecated: A <% B (#6500)
- Use instead: (implicit ev: A => B)
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.
Ah, good to know
_outerField: MappedField[JoinTypeA, OuterMapper], | ||
_innerField: MappedField[JoinTypeA, InnerMapper], | ||
qp: Zoom* | ||
)(implicit ev: Zoom => QueryParam[InnerMapper]): InThing[OuterMapper] = { |
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.
Looks like we've also added implicits here that I'm not totally clear on the use of.
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.
As previously described, the deprecated view bound syntax was replaced by implicit function.
- View bounds deprecated
- Deprecated: A <% B (#6500)
- Use instead: (implicit ev: A => B)
Sorry it took so long for me to get around to this. Personal life has been a bit of a mess lately so I appreciate the patience. I've left some questions about some implicits I saw added in particular. Otherwise things look Fine™, but I'm not a lift-mapper expert so I'm not sure I'm the best judge. I'd love to know how confident you are that the changes you made are covered by tests. |
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’m good with these changes. Will need conflict resolution before merging. Happy to do that at my next opportunity.
False alarm, disregard. |