search

issuu / ocaml-protoc-plugin

ocaml-protoc-plugin
https://issuu.github.io/ocaml-protoc-plugin/
Other
48 stars 19 forks source link

Derive std for well-known Google types #23

Closed Leonidas-from-XIV closed 3 years ago

Leonidas-from-XIV commented 3 years ago

This is required for everybody that uses the Google types and wants to make them [@@derive show] or any other deriver.

andersfugmann commented 3 years ago

I'm not very fond of adding more dependencies to the runtime. Potentially we could create a new library called something like ocaml-protoc-plugin.google_types_deriving to offer prebuild google types with derivers. Until then I think the users of the library should make their own google types if they need annotations on the type signatures.

Leonidas-from-XIV commented 3 years ago

In such case I would ask the question, why does the library have google types in any case, since they are unlikely to be in a form that the consumer would want, yet will pay for them in code size by having them included in the library?

Would it not be better to split them out completely, into a library called ocaml-protoc-plugin.google_types that depends on ppx_deriving and has these helpers built-in?

andersfugmann commented 3 years ago

I added them for completeness, but you are right; they do belong in separate package. And if we go down that route, it would be trivial to add two variants - one with deriving and one without.

Although dune allows us to do conditional compilation based on installed packages, I think it adds too much magic if we were to compile the well known types with deriving if that package is installed.

Leonidas-from-XIV commented 3 years ago

Although dune allows us to do conditional compilation based on installed packages, I think it adds too much magic if we were to compile the well known types with deriving if that package is installed.

I agree, optional dependencies are considered a misfeature of OPAM, since packages which depend on the "deriving" bit would need to require the Google types as well as depending on the package that triggers the additional functionality (ppx_deriving) without necessarily actually depending on it (in a similar way how programs wanting to use cohttp with HTTPS have to also depend on ssl/tls without actively requiring the libraries themselves).