teamwalnut / graphql-ppx

GraphQL language primitives for ReScript/ReasonML written in ReasonML
https://graphql-ppx.com
MIT License
257 stars 53 forks source link

Opam releases #250

Closed gf3 closed 3 years ago

gf3 commented 3 years ago

Will the latest releases be published to opam as well?

jfrolich commented 3 years ago

Not really, I didn't really look into publishing to opam yet, but a PR would definitely be welcome!

gf3 commented 3 years ago

@jfrolich i'm pretty new but i'd be happy to take a stab at this!

anmonteiro commented 3 years ago

Hey @gf3, great to see you here. I used secretary in anger back in the day 🙂

These days doing a release to OPAM should be mostly straightforward with dune-release.

Jaap has been tagging releases as he pushes them to NPM, so running dune-release on one of these git tags shouldn't be to hard. Happy to help out along the way

gf3 commented 3 years ago

Hey @gf3, great to see you here. I used secretary in anger back in the day slightly_smiling_face

hahaha that feels like a long time ago

These days doing a release to OPAM should be mostly straightforward with dune-release.

Jaap has been tagging releases as he pushes them to NPM, so running dune-release on one of these git tags shouldn't be to hard. Happy to help out along the way

thanks for the heads up, i'll start with dune-release, cheers!

gf3 commented 3 years ago

@anmonteiro i've created a PR (above) to make it easier to call dune-release however the project still requires a CHANGELOG.md or CHANGES.md to function properly

pitag-ha commented 3 years ago

It would really be great to have this released on opam, among others so that OCaml folks also have a ppxlib compatible graphql_ppx version! In some projects, they even have to vendor graphql_ppx currently as there isn't any ppxlib compatible release, see e.g. here.

Concerning the CHANGES.md file: for the dune-release publish command, dune-release has an argument called --change-log to pass in a changelog path that's not at the root and/or not called CHANGES.md (such as your documentation/docs/changelog.md). But for simplicity and since graphql_ppx hasn't been released on opam for the last 3 releases, what about the following: you (or if needed, I can do that and open a PR) create a file called CHANGES.md at the root of the project, you copy the entries of sections 1.2.0, 1.1.0 and 1.0.2 from documentation/docs/changelog.md into it (after the 1.2.0 "header"). Then, after opam installing dune-release, you simply checkout the v1.2.0 tag and from within hte project run

dune-release check

and if everything is ok you run

dune-release

(since you already have the release tag, you don't even need dune-release tag before) If that's your first release with dune-release you'll be prompted some config information such as the url of your opam-repository fork and the path to your opam-repository clone. But apart from that everything else should be done automatically. What do you think @jfrolich (I'm assuming that you're in charge of this, since you seem to have done the last releases. Please let me know if I'm wrong.)?

Btw, thanks for the work you've done to get this released on opam, @gf3!

pitag-ha commented 3 years ago

Btw, aside the obvious advantage of making graphql_ppx available also to OCaml users, there's another advantage if you release this on opam: whenever we bump the ppxlib AST, we'd take graphql_ppx into account and send a patch if we break it with the AST bump.

anmonteiro commented 3 years ago

@pitag-ha Thanks for the detailed comment!

I think it makes sense to keep releasing to opam. Honestly we haven't done it because most folks consume this ppx from NPM, and myself from my own nix overlays.

Starting with a PR for the changelog makes sense. I could probably do the opam releases myself later on.

pitag-ha commented 3 years ago

Thanks @anmonteiro!

By the way, we've been talking a lot about the changelog, but in fact to publish an already existing release on opam, a changelog isn't necessary. You could just use the release tag tarball and run (thanks @kit-ty-kate for that info!):

opam publish -v 1.2.0 https://github.com/reasonml-community/graphql-ppx/archive/refs/tags/v1.2.0.tar.gz

or, with dune-release, I think (if v1.2.0 is checked out) (edit: I think the dune-release opam pkg command wouldn't work just like that. You'd also have to download the release tarball and hand in its path via the --dist-file argument.)

dune-release opam pkg --dist-url https://github.com/reasonml-community/graphql-ppx/archive/refs/tags/v1.2.0.tar.gz 
dune-release opam submit

should do.

Does that sound good to you? That being said, I can still open a PR for the changelog so that you can use dune-release for the whole release process in the future, in particular to also have a release with release notes on github, if you want.

pitag-ha commented 3 years ago

About the question if to use opam publish or dune-release: When doing a whole release with dune-release, dune-release is usually quite easy to use. But in this concrete situation, opam publish is the easier choice. But, also in this situation, dune-release follows better practices, in particular it adds a hash of the release tarball to the release meta data (that's also why you need to hand a release tarball path in via the --dist-file argument, when running dune-release opam pkg without having run dune-release distrib before). Adding the hash is a good practice so that opam can verify the tarball when installing the package.

kit-ty-kate commented 3 years ago

About the question if to use opam publish or dune-release: When doing a whole release with dune-release, dune-release is usually quite easy to use. But in this concrete situation, opam publish is the easier choice. But, also in this situation, dune-release follows better practices, in particular it adds a hash of the release tarball to the release meta data (that's also why you need to hand a release tarball path in via the --dist-file argument, when running dune-release opam pkg without having run dune-release distrib before). Adding the hash is a good practice so that opam can verify the tarball when installing the package.

I’m not sure to follow your distinction. Both add a hash of the tarball, the only difference in that respect is that by default (so when calling dune-release without arguments, not our case here) dune-release generates its own tarball and uploads it to github. opam-publish on the other hand simply takes the tarball given as arguments or the one generated by github if no argument is given.

pitag-ha commented 3 years ago

Both add a hash of the tarball

Yes, you're absolutely right. Thanks! I mixed things up with the commit hash which is only added by dune-release, but not by opam publish (correct me if I'm wrong again). dune-release adds it since that's helpful for vendoring tools.

pitag-ha commented 3 years ago

@anmonteiro, do you want me to try opam publish?

kit-ty-kate commented 3 years ago

I just opened https://github.com/ocaml/opam-repository/pull/19154 to get it started (with 1.2.2 released yesterday), if anyone needs help for releasing next time please ping me on Discord or wherever

anmonteiro commented 3 years ago

I’m unavailable until the end of next week, without access to a computer. Will check in then.

kit-ty-kate commented 3 years ago

Merged. This issue can now be closed. Can someone return the fixes I've made to the opam file in the master branch?

jfrolich commented 3 years ago

@kit-ty-kate: can you make a PR? Much appreciated!

Zimmi48 commented 3 years ago

most folks consume this ppx from NPM, and myself from my own nix overlays.

@anmonteiro No need for a Nix overlay, the package is already available in ocamlPackages (see https://search.nixos.org/packages?channel=unstable&from=0&size=50&sort=relevance&query=graphql_ppx). Current packaged version is 1.2.0 but feel free to open a PR bumping it to 1.2.2 if you need it.