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

Upgrade to OCaml 4.08+beta2 #70

Merged
merged 3 commits into from
Jun 19, 2019
Merged

Upgrade to OCaml 4.08+beta2 #70

merged 3 commits into from
Jun 19, 2019

Conversation

jhwoodyatt
Copy link
Contributor

@jhwoodyatt jhwoodyatt commented Mar 14, 2019

For reference, this is what OCaml 4.08+beta2 seems to require (issue #69).

@jhwoodyatt
Copy link
Contributor Author

I don't understand why Travis CI is complaining, and I don't have the time to investigate.

@hhugo
Copy link
Contributor

hhugo commented Mar 28, 2019

cc @xclerc

@kit-ty-kate
Copy link
Collaborator

@jhwoodyatt I opened a PR on your fork to fix travis: jhwoodyatt#1

@kit-ty-kate
Copy link
Collaborator

all green. cc @alainfrisch @gasche (not sure who's maintaining this)

@gasche
Copy link
Contributor

gasche commented Jun 17, 2019

I know I'm not, but maybe I can try to have a look later in the week if nobody has stepped up. (cc @alainfrisch @nojb @xclerc @diml)

Copy link

@nojb nojb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I don't have merge rights though.

@gasche
Copy link
Contributor

gasche commented Jun 19, 2019

It's embarrassing that I don't know how to check the list of people with merge rights (to me or to the Github interface), but it may be that only @alainfrisch is in this set. I would warmly recommend to add @nojb to it, though :-)

@Drup Drup merged commit 331fbc4 into ocaml-ppx:master Jun 19, 2019
@Drup
Copy link
Collaborator

Drup commented Jun 19, 2019

I have merge rights! But I can't add more people to the repository.
I also have very little time right now, so I won't be able to make a release myself for a few weeks.

@jhwoodyatt jhwoodyatt deleted the x-4.08 branch June 19, 2019 16:52
@jhwoodyatt jhwoodyatt mentioned this pull request Jun 19, 2019
@gasche
Copy link
Contributor

gasche commented Jun 26, 2019

While testing the master branch on 4.08 I found a small issue -- the generated code still mentions Pervasives -- for which I propose a fix in #72. Would someone look at it and consider merging it?

@XVilka
Copy link

XVilka commented Jun 28, 2019

Shouldn't be this merged one already? It is pretty obvious it is good one.

@hhugo
Copy link
Contributor

hhugo commented Jul 7, 2019

I believe metaquot is currently broken with this feature.
One should ignore pexp_loc_stack, ptyp_loc_stack, ppat_loc_stack when generating patterns on the ast. see what's done for location and attributes: https://github.com/ocaml-ppx/ppx_tools/blob/master/ppx_metaquot.ml#L175

Also see the patch in ppx_tools_versionned https://github.com/ocaml-ppx/ppx_tools_versioned/pull/22/files#diff-7ab097557bb7183454f885e4595acc38R180

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.

7 participants