Skip to content

Conversation

@bramford
Copy link
Contributor

@bramford bramford commented Jul 17, 2019

gremlin.0.1.1

Gremlin Client Library
This is an Apache Tinkerpop3 Gremlin client library.

See the official tinkerpop3 and gremlin docs:

This client library is implemented following the driver provider requirements:



🐫 Pull-request generated by opam-publish v2.0.0

@bramford bramford mentioned this pull request Jul 17, 2019
@kit-ty-kate
Copy link
Member

You forgot to provide the url to the archive of your package (see the url block in other opam files)

#=== ERROR while compiling gremlin.v0.1.1 =====================================#
# context              2.0.4 | linux/x86_64 | ocaml-base-compiler.4.08.0 | pinned
# path                 ~/.opam/4.08/.opam-switch/build/gremlin.v0.1.1
# command              ~/.opam/4.08/bin/dune build -j 2 -p gremlin
# exit-code            1
# env-file             ~/.opam/log/gremlin-15-b64e6b.env
# output-file          ~/.opam/log/gremlin-15-b64e6b.out
### output ###
# Error: I don't know about package gremlin (passed through --only-packages/--release)

@kit-ty-kate
Copy link
Member

Also, could you avoid the v prefix to the version number? With the notable exception of janstreet packages, there is no need for it and it's rather unwanted and source of bug.

"conduit-lwt-unix"
"containers"
"core"
"dune"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"dune"
"dune" {>= "1.10"}

Your dune-project file in the source code says you require at least dune 1.10.

@kit-ty-kate
Copy link
Member

You have a lot of big dependencies with no constraints on their minimal version required. I suspect there are several needed here but if you don't have time to put them all it's ok.

@kit-ty-kate
Copy link
Member

wait you had a whole lot of constraints in #14498, where did there all go?

@kit-ty-kate
Copy link
Member

Also I'm not entirely sure why you removed this PR to create another one. It's more difficult to work with several PRs than with just one if you push over it.

@camelus
Copy link
Contributor

camelus commented Jul 17, 2019

☀️ All lint checks passed 9f7c9e1
  • These packages passed lint tests: gremlin.0.1.1

☀️ Installability check (+1)
  • new installable packages (1): gremlin.0.1.1

@bramford bramford changed the title Package gremlin.v0.1.1 Package gremlin.0.1.1 Jul 22, 2019
@bramford
Copy link
Contributor Author

bramford commented Jul 22, 2019

Everything should be working now.

wait you had a whole lot of constraints in #14498, where did there all go?

I've put them back in. I wasn't sure whether to specify the known working dependency versions or leave them blank and let others test/report. I've seen various packages from the community do it either way.

Also I'm not entirely sure why you removed this PR to create another one. It's more difficult to work with several PRs than with just one if you push over it.

This was because github doesn't allow branch renaming in PRs as far as I could tell. I wanted the branch name to match the version number.

@kit-ty-kate
Copy link
Member

Your package is again missing the url field

@bramford
Copy link
Contributor Author

Your package is again missing the url field

Fixed.

@kit-ty-kate
Copy link
Member

Thanks!

@kit-ty-kate kit-ty-kate merged commit 05d2faf into ocaml:master Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants