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

[bug fix] Import module types with optional arguments #37

Merged

Conversation

thierry-martinez
Copy link
Contributor

When importing a module type containing a function value taking an
optional argument of type t, the imported type for the argument was
t option instead of t.

When importing a module type containing a function value taking an
optional argument of type `t`, the imported type for the argument was
`t option` instead of `t`.
@whitequark whitequark merged commit 0a523ec into ocaml-ppx:master May 13, 2019
@MattWindsor91
Copy link

Sorry if this isn't the right place to ask, but are there any plans to cut an opam release with this bugfix? Asking because I just ran into precisely this bug myself when using the opam release of ppx_import! 🙂

@ejgallego
Copy link
Collaborator

Hi @MattWindsor91 , sure, I'll push a release right now. @whitequark this PR was missing a CHANGES.md entry, IMVHO maybe we should require PRs to always modify that? [We could add a CI check for example]

@whitequark
Copy link
Contributor

maybe we should require PRs to always modify that? [We could add a CI check for example]

Seems heavy-handed... Not sure what I think about it.

@gasche
Copy link
Contributor

gasche commented Jun 16, 2019

For smaller projects like this one I've been writing release notes myself at release time, mentioning only the PR that were not purely internal (that I thought may affect the users of the software). It's generally 3/4 things to mention each release, so fine. It's less hassle than forcing the users to do this -- of course if users include a Changelog entry in their PR, that's swell too.

mseri pushed a commit to ocaml/opam-repository that referenced this pull request Jun 18, 2019
* [new release] ppx_import (1.6.2)

CHANGES:

* Fix import of module types with optional arguments
    (Thierry Martinez ocaml-ppx/ppx_import#37, review by whitequark)

* ppx_import.1.6.2: remove redundant fields
fdopen added a commit to fdopen/opam-repository-mingw that referenced this pull request Jun 19, 2019
* [new release] ppx_import (1.6.2)

CHANGES:

* Fix import of module types with optional arguments
    (Thierry Martinez ocaml-ppx/ppx_import#37, review by whitequark)

* ppx_import.1.6.2: remove redundant fields
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.

5 participants