Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Elm] Add Elm client generator #6947

Merged
merged 1 commit into from
Dec 20, 2017
Merged

[Elm] Add Elm client generator #6947

merged 1 commit into from
Dec 20, 2017

Conversation

eriktim
Copy link
Contributor

@eriktim eriktim commented Nov 13, 2017 •
edited
Loading

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This is a work-in-progress, but @wing328 suggested me to open this PR in an early stage for feedback and support.

This PR adds support for the elm-lang. I'm already testing it on a ~2000 lines swagger-file and the basics are already working. What follows is a (mutable) TODO list:

  • Get rid of the vendor extensions;
  • Add support for path variables;
  • Add support for additionalProperties/Dicts;
  • Add support for Date/DateTime;
  • Add support for ByteArray/Binary;
  • Add support for authentication;
  • Distinct between Int and Float;
  • Limit the exposure of modules;
  • Support recursive types;
  • Try minimizing/removing dependencies (elm-decode-pipeline, elm-date-extra);
  • Double check elm-format is applied correctly;
  • General clean-up.

Most of them are quite Elm-specific, but for example the vendor extensions I use are just needed to define some decoder/encoder name. This feels kinda hacky, but I am not sure how to do so correctly. Please let me know whether I am the right track.

@wing328 wing328 added this to the v2.3.0 milestone Nov 13, 2017
@eriktim eriktim changed the title WIP - Add Elm language [Elm] WIP - Add Elm language Nov 13, 2017
@wing328
Copy link
Contributor

wing328 commented Nov 27, 2017

Related discussion in Reddit: https://www.reddit.com/r/elm/comments/7d7jwq/swagger_codegen_for_elm/

@wing328 wing328 modified the milestones: v2.3.0, v2.4.0 Dec 18, 2017
@eriktim
Add Elm language - BETA
bc9fa48 
This is the initial Codegen for Elm 0.18.
Please try it out and provide feedback.

Not yet supported:

* path variables;
* additionalProperties;
* authentication;
* recursive types.
@wing328
Copy link
Contributor

wing328 commented Dec 20, 2017

As discussed, let's merge this into master as a beta release so as to collect more feedback from Elm developers.

@wing328 wing328 merged commit d6539ba into swagger-api:master Dec 20, 2017
@wing328 wing328 mentioned this pull request Dec 20, 2017
Open
12 tasks
@wing328
Copy link
Contributor

wing328 commented Dec 20, 2017

I've created to #7224 to track the enhancements.

@wing328 wing328 changed the title [Elm] WIP - Add Elm language [Elm] Add Elm client generator Dec 20, 2017
@wing328 wing328 modified the milestones: v2.4.0, v2.3.0 Dec 20, 2017
@wing328
Copy link
Contributor

wing328 commented Dec 20, 2017

Tweet to promote the new generator: https://twitter.com/wing328/status/943494382203715584

slarti-b added a commit to slarti-b/swagger-codegen that referenced this pull request Dec 26, 2017
@slarti-b
Merge branch 'master' into optional-params-with-delegate
2af47ab 
* master:
  move bvwells to go tech comm
  update current stable version in readme
  skip push snapshot to avoid error
  comment out checkstyle in circleci pom.xml
  comment out check style plugin
  Revert "update version to 2.4.0-SNAPSHOT"
  update ci config to install codegen locally
  update doc to 2.3.0
  update version to 2.4.0-SNAPSHOT
  update version to 2.3.0
  fix net35 with JsonSubTypes (swagger-api#7043)
  [Javascript] Set ES5 as default (swagger-api#7239)
  swagger-api#7201: Take the modelPropertyNaming property into account again (swagger-api#7202)
  add trenneman as elm creator
  add bvwells to go technical committee
  Video "Building an API with Swagger" (swagger-api#7237)
  [Akka-Scala] This is a fix to a bug introduced by this PR swagger-api#7172 (swagger-api#7228)
  Add Elm language - BETA (swagger-api#6947)
  bump stack resolver to lts-10.0 (swagger-api#7221)
@ilyakard
Copy link

This is really great, thanks guys! Managed to parse the FHIR API specification, which is pretty huge.

@andys8
Copy link

andys8 commented May 7, 2018

@trenneman What a great PR. It looks very promising. I started to look into it. I found smaller issues and opened issues for them. The changes are addressing formatting issues and refer to elm-format. Would be great to have the output matching the formatting standard. I tried to make out the files related to the issues. Hope I could help a little, even though those are small changes.

Issues: #8151 and #8152
PR: #8150

@eriktim eriktim deleted the elm-lang branch May 11, 2018 09:41
@eriktim
Copy link
Contributor Author

eriktim commented May 11, 2018

Thanks @andys8. Adhering to elm-format was already the intention, but isn't that simple in all cases. Are you going to file a PR for the other issues as well or can they be picked up?

@andys8
Copy link

andys8 commented May 11, 2018

I'm not planning to file PRs for now. So they're free to be picked up. I want to dig more into swagger codegen in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Client: Elm Enhancement: New generator
Projects
None yet
Milestone
v2.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@eriktim @wing328 @ilyakard @andys8