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

Fix #27 - Support for OCaml 4.08 #26

Merged
merged 4 commits into from
Aug 1, 2019
Merged

Conversation

XVilka
Copy link
Contributor

@XVilka XVilka commented Jun 26, 2019

No description provided.

@XVilka XVilka mentioned this pull request Jun 26, 2019
@gasche
Copy link
Contributor

gasche commented Jun 26, 2019

Currently the 4.08 is not passing. I'll merge the 4.07 change, and then we should fix 4.08 before enabling the CI for it. (This means you might have to rebase your PR to fix the conflict with the 4.07 one, sorry.)

@XVilka
Copy link
Contributor Author

XVilka commented Jun 26, 2019

Surely, no problem, it is trivial change anyway.

@XVilka
Copy link
Contributor Author

XVilka commented Jul 10, 2019

Build should be restarted/retriggered now, the ppx_deriving was updated.

@gasche
Copy link
Contributor

gasche commented Jul 15, 2019

Thanks for importing the fix here. I would be happy to merge if the CI agree, and then can work on releasing a new version.

@XVilka
Copy link
Contributor Author

XVilka commented Jul 15, 2019

@gasche it is for attr_loc, the third record field: http://caml.inria.fr/pub/docs/manual-ocaml/compilerlibref/Parsetree.html

@gasche
Copy link
Contributor

gasche commented Jul 15, 2019

I don't think that works.

# type t = { x : int; y : int; z : int };;
type t = { x : int; y : int; z : int; }
# fun { x; _; _ } -> x;;
Error: Syntax error: '}' expected
# fun { x; y = _; _ } -> x;;
- : t -> int = <fun>

In a record pattern _ ; means "ignore all the other fields", you cannot ignore fields implicitly by position.

@XVilka
Copy link
Contributor Author

XVilka commented Jul 15, 2019

Apparently Pberr_wrong_attr {{txt; loc}; _} doesn't work:

File "ppx_deriving_protobuf.cppo.ml", line 148, characters 22-23:
# Error: Syntax error

Update: Lol, my bad, didn't notice the field names are different

@gasche
Copy link
Contributor

gasche commented Jul 15, 2019

Well I guess you also need to name the field at which the sub-record { txt; loc } occurs.

@XVilka
Copy link
Contributor Author

XVilka commented Jul 15, 2019

This one solved, but there are more. Will continue tomorrow locally, seems no easy one-line blind fix. Sorry for the noise, thought lazy GitHub pencil edit would solve this.

@XVilka
Copy link
Contributor Author

XVilka commented Jul 16, 2019

@gasche ok, now it is green and ready.

@XVilka XVilka changed the title Add autobuilds for OCaml 4.08 Support for OCaml 4.08 Jul 16, 2019
@XVilka XVilka changed the title Support for OCaml 4.08 Fix #27 - Support for OCaml 4.08 Jul 17, 2019
@XVilka
Copy link
Contributor Author

XVilka commented Jul 22, 2019

@gasche can you please take a look? I think it is ready to be merged

@XVilka
Copy link
Contributor Author

XVilka commented Jul 31, 2019

Is there anything that should be done to get this merged?

Copy link
Contributor

@gasche gasche left a comment

Choose a reason for hiding this comment

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

Apologies for the delay and thanks for the pings from time to time. This isn't quite ready but the high-level plan is of course to get it merged eventually, and then make a release.

src/ppx_deriving_protobuf.cppo.ml Outdated Show resolved Hide resolved
Copy link
Contributor

@gasche gasche left a comment

Choose a reason for hiding this comment

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

Seems good to merge when CI agrees, thanks! (Minor comment inline.)

src/ppx_deriving_protobuf.cppo.ml Outdated Show resolved Hide resolved
@XVilka
Copy link
Contributor Author

XVilka commented Aug 1, 2019

Seems Travis is dead. You might want to restart failed jobs.

@gasche
Copy link
Contributor

gasche commented Aug 1, 2019

I restarted some jobs; note that the 4.05 failure is real:

# File "ppx_deriving_protobuf.cppo.ml", line 391, characters 10-19:
# Error: Unbound constructor Rtag_patt

@XVilka
Copy link
Contributor Author

XVilka commented Aug 1, 2019

Ok, now everything is green, except Travis own failure on 4.05 (can be restarted).

@gasche gasche merged commit 4a28310 into ocaml-ppx:master Aug 1, 2019
@gasche
Copy link
Contributor

gasche commented Aug 1, 2019

Thanks! Merged.

@XVilka
Copy link
Contributor Author

XVilka commented Aug 2, 2019

Would be nice to make a release after that.

@gasche
Copy link
Contributor

gasche commented Aug 9, 2019

(See #29 for the release-preparation thread.)

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.

None yet

2 participants