search

andrenth / ocaml-swagger

Swagger 2.0 code generator for OCaml
37 stars 10 forks source link

Rename "create" to "make" in generated code #4

Closed rizo closed 5 years ago

rizo commented 6 years ago

As discussed in https://github.com/andrenth/ocaml-swagger/pull/1#issuecomment-414979522 this renames "create" into "make" in the generated code.

I tried integrating ppx_deriving make plugin but quickly realised that it would be difficult (if not impossible) to do so. It would work for implementations but not for signatures, where the type is abstract. The limitation is that deriving make requires a non-abstract record type to generate the constructor function.

I'm not sure if there is a programatic way to ask ppx_deriving to generate the "make" function. I started a discussion on the forum for this: https://discuss.ocaml.org/t/deriving-make-for-abstract-types-in-signatures/2548.

An alternative would be to not use abstract representation for record types and instead mark them as private. Not sure if it's a good idea.

I also wanted to add that I'm very happy with your Kubernetes library generated with ocaml-swagger. My main motivation to rename "create" into "make" is based on this small preprocessor that simplifies the configuration syntax for large Kubernetes definitions.

Oh and I also started adding some comments, hope that's ok.

rizo commented 5 years ago

Do you have any thoughts on this? I'm happy to drop this PR if you'd prefer to keep the current naming convention.

andrenth commented 5 years ago

Hi. Thanks for the PR and sorry for being slow to reply. I haven't been using OCaml at work recently, so it's been really hard to find the time for the project.

rizo commented 5 years ago

Thank you!

And sorry for insisting, I didn't mean to bother. Could we arrange a new release with the recent changes? I don't mind helping you with that.

andrenth commented 5 years ago

So, I'm trying to prepare a new release, but on OCaml 4.05.0 and 4.06.0 with dune 1.4.0 I see a lot of warnings from unused variables from the atdgen auto-generated code... With 4.07.0 everything seems to be fine.

Have you seen something like that?

rizo commented 5 years ago

Dune recently switched to running builds in dev profile by default. In that mode all warnings are treated as errors. It is very strict (which is useful most of the times).

I opened a PR to fix all warnings by adding compiler annotations to ignore some of them and explicitly changing some bits for others.