cretz / pb-and-k

Kotlin Code Generator and Runtime for Protocol Buffers
MIT License
139 stars 15 forks source link

pure kotlin dependency-free Marshaller/Unmarshaller and native #15

Open phcoder opened 5 years ago

phcoder commented 5 years ago

This provides native variants for all the components.

Code is adapted from java protobuf code.

cretz commented 5 years ago

That's a pretty significant level of effort, thanks. Notes:

  1. I think at a higher level saying you referenced the Google protobuf lib Java reference impl is reasonable over embedding the copyright headers.
  2. I don't think we should remove the common Marshaller, Unmarshaller, and Sizer. I don't want the "pure" impl to take over the other ones, just be an option instead of them. Also, I would like backwards compatibility (at least mostly).
  3. Same with Util...some platforms have faster ways of doing this, I don't want to force the pure Kotlin one on those platforms.
  4. Do you mind if instead of accepting this PR wholesale, if I work some of it into a new PR that takes a lighter approach? I will gladly give you credit in the README for the initial impl. I would essentially make a runtime/runtime-pure cross platform project, a runtime/runtime-native project that uses it, then conformance and protoc-gen-kotlin set of native versions. After that, maybe even runtime/runtime-pure-js and runtime-pure-jvm if Kotlin MPP doesn't support using a common-only project directly.
phcoder commented 5 years ago
cretz commented 5 years ago

Thanks for the response. I have to admit I don't have the immediate time to incorporate this but hopefully I can revisit it soon.

phcoder commented 5 years ago

If you want I can do the changes you requested and update this PR

phcoder commented 5 years ago

I added some flexibility and changed it to use JS/JVM tools on those platforms. By looking through protobufjs it looks like it implements same things as kotlin-pure code and doesn't get advantage of any js functions. So it feels like protobufjs doesn't provide any value. Should we make js and native both use kotlin pure?

LiewJunTung commented 5 years ago

Writing this to show my support for this feature! 🙌

cretz commented 5 years ago

@LiewJunTung and @phcoder - Sorry I haven't done more here, I haven't been using Kotlin as much lately in my day job. In general I would take this and make a set of "pure" projects to not disturb the existing projects. Can't make any promises when I can revisit though, sorry.