Adds more tests around the macros for trait edge cases and others #1725
Conversation
|
Does a bunch of damage on #1721 but there's more warnings, some of which the linter fires in generated code, not too sure how to stop that even.... @johnynek As you requested :) (Also included is failing test + bug fix for some trait handling for classes with lots of fields failing to deserialize. And some better escape hatch handling so users can supply their own OrderedSerializations when the macros fail more often). |
| implicit def ordSer[T]: OrderedSerialization[T] = macro com.twitter.scalding.serialization.macros.impl.OrderedSerializationProviderImpl[T] | ||
| } | ||
|
|
||
| object BinaryOrdering extends BinaryOrdering {} |
There was a problem hiding this comment.
I don't think you need {}
| .collect { case m: MethodSymbol => m } | ||
| .filter(m => m.name.toString.startsWith("_")) | ||
| .filter(m => m.name.toTermName.toString.startsWith("_")) |
There was a problem hiding this comment.
I had to remove a bunch of these since they were calling toTermName on things that were already TermName, but I guess with blackbox this changed?
There was a problem hiding this comment.
I suspect i've glossed over some of those warnings migrating in bulk the code with the fixes in it. I'll do a clean compile or two to make sure i've not added new warnings.
|
Ok i think that should be better minus those few fixes. There are a bunch of warnings about unreachable code in match statements, though they seem spurrious. I've added a sys.error into one of them and it fires... |
|
I'm merging as is and we can deal with the blackbox issue in a follow up (I would love to get the warnings cleaned up in scalding). |
This should fail in CI, will follow up with code changes to remove warnings and fix these tests.