Ordered Serialization macros for thrift #1338
Conversation
| * rev: ? | ||
| * built at: ? | ||
| */ | ||
| package com.twitter.scalding.thrift.macros.scalathrift |
There was a problem hiding this comment.
is there any way we could not check in generated code?
There was a problem hiding this comment.
@ianoc Do you still have the original thrift file? We will have to generate these on the fly if we dont want to check them in.
There was a problem hiding this comment.
I'd rather just use the scrooge plugin and generate these, personally (also it is a better test, because scrooge will evolve).
| /** | ||
| * @author Mansur Ashraf. | ||
| */ | ||
| trait RequiredBinaryComparators extends RequiredBinaryComparatorsConfig { |
There was a problem hiding this comment.
Can you add a comment to make it clear that this is here for people that use scrooge, but otherwise is identical to the one in scalding-core.
|
Looks good to me other than the minor confusion I hit when I saw the duplicated RequiredBinaryComparators. |
|
@johnynek let me know if you are okay to merge |
Ordered Serialization macros for thrift
continuation of #1210