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.

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

4.05 support, bug fixes and a few minor functionality updates #67

Closed
wants to merge 1 commit into from
Closed

Conversation

stevebleazard
Copy link
Contributor

  • 4.05 support
  • add let _ = to_yojson / of_yojson to generated code to avoid warnings when they aren't used
  • Fix bug where doing [@@deriving of_yojson] causes an unused rec warning
  • Add support for keys function similar to Fields.name. Module Yojson_fields{_type} created with a keys value of a string list containing all the keys
  • from_yojson_exn created to raise an exception rather than return an error

…en they

  aren't used
- Fix bug where only doing [@@deriving of_yojson] causes an unused rec - warning
- Add support for keys function similar to Fields.name. Modules named
  Yojson_fields{_type} with a keys value of string list
- from_yojson_exn created to raise an exception rather than return an
  error
@gasche
Copy link
Contributor

gasche commented Nov 17, 2017

Could you split the separate changes in separate PRs? I think the 4.05 support is not the right approach -- #66 is better and should be merged soon, but it relies on changes on the side of ppx_deriving, but the other features should be considered on their own.

@gasche gasche closed this Nov 21, 2017
@gasche
Copy link
Contributor

gasche commented Nov 21, 2017

@stevebleazard the 4.05 support part of the patch is now obsolete (ppx_deriving.4.2.1 was released with a different API on 4.05 and 4.06). I would welcome your other changes, preferably split as separate commits in separate PRs. If you can't work of this, please let me know.

@stevebleazard
Copy link
Contributor Author

stevebleazard commented Nov 22, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants