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

graphql_parser.0.11.0: fix missing result dependency. #13803

Merged
merged 1 commit into from
Apr 1, 2019

Conversation

dbuenzli
Copy link
Contributor

@dbuenzli dbuenzli commented Apr 1, 2019

It seems graphql_parser.0.11.0 is getting its result dependency indirectly through fmt. But the latest release of fmt drops the result dependency which results in a build failure.

This PR adds a dependency on result (note that e.g. 0.9.0 has the dependency on `result).

/cc @andreas

@dbuenzli dbuenzli mentioned this pull request Apr 1, 2019
@andreas
Copy link
Contributor

andreas commented Apr 1, 2019

Sorry about that, and thanks for the fix. I'll make sure it's fixed with next release.

@mseri mseri merged commit a4d2989 into ocaml:master Apr 1, 2019
@mseri
Copy link
Member

mseri commented Apr 1, 2019

Thanks

@camelus
Copy link
Contributor

camelus commented Apr 1, 2019

☀️ All lint checks passed 8439b65
  • These packages passed lint tests: graphql_parser.0.11.0

☀️ Installability check (10916 → 10916)

@phated
Copy link
Contributor

phated commented Apr 1, 2019

This is now causing my build to fail. I'm not sure why. I'm using ocaml 4.06 so it shouldn't be an issue, right?

Edit: Forcing the dependency on 0.9.0 makes it work again.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Apr 1, 2019

@phated without failure details it's going to be hard to say anything.

@dbuenzli dbuenzli deleted the fix-graphql_parser-result branch April 1, 2019 22:16
@dbuenzli
Copy link
Contributor Author

dbuenzli commented Apr 1, 2019

Also it seems this was merged after the fmt PR. I should have mentioned it should have been merged before. Are you sure you were not witnessing the state with fmt in and that one not in ?

@phated
Copy link
Contributor

phated commented Apr 1, 2019

It was using fmt at 0.8.6 which I believe is the latest. The error is just "Error: Unbound module Result" but I'm not sure if the error log is helpful since it's from esy.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Apr 1, 2019

It was using fmt at 0.8.6 which I believe is the latest. The error is just "Error: Unbound module Result" but I'm not sure if the error log is helpful since it's from esy.

Precisely this PR fixes this error.

@phated
Copy link
Contributor

phated commented Apr 1, 2019

But I was on graphql_parser 0.11.0 unless this overwrote an 0.11.0 version instead of incrementing the version?

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Apr 1, 2019

But I was on graphql_parser 0.11.0 unless this overwrote an 0.11.0 version instead of incrementing the version?

Yes it did. It seems we still don't have a good answer to this see #10531

@samoht
Copy link
Member

samoht commented Apr 2, 2019

I don't understand how this PR is fixing the issue ; result should be added to the build files as well as the opam file, right?

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Apr 2, 2019

Humpf. Right, to both. I'll PR a constraint.

@dbuenzli
Copy link
Contributor Author

dbuenzli commented Apr 2, 2019

See See #13808

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.

6 participants