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

Specify package version in opam file #1445

Closed
wants to merge 2 commits into from
Closed

Conversation

clembu
Copy link

@clembu clembu commented Aug 7, 2020

A lot of projects will tend to either pin ocamlformat to a certain commit or add constraints on its version in their dependency to have project-wide formatting consistency and sometimes specific features (like the recent .eliom support for example).

When these pins or constraint change, pulling the changes doesn't change the version of the package because it doesn't come from the opam versioned repo but from a git repo somewhere, and this file here will be read to determine the version and check it against the constraint.

I propose to maintain this version number here for every package version published, to avoid problems with pins and constraints in the future.
Unless you're alright with projects having to uninstall and reinstall ocamlformat everytime they need to change their version, of course.

Another possibility is to specify "dev" as the version long as it's not an actual version published on opam, and change it for the release candidates on their own branch. I'm not sure which is better.

@facebook-github-bot
Copy link

Hi @FacelessPanda!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@clembu
Copy link
Author

clembu commented Aug 7, 2020

check failed so I added a line in the changelog, before seeing that a changelog not needed label existed. Should I keep the line in the log or remove it?

@emillon
Copy link
Collaborator

emillon commented Aug 24, 2020

Hi,

We're trying to move to a simpler system where version is sourced from the git repo (#1133) and opam files are generated (#1361), so this feel a bit like a step backwards to add the version number in the opam file.

But I agree that there's no good way to set a version number when ocamlformat is part of a monorepo or more generally when its sources are not pulled from a release tarball or this git repository.

One thing we can do is add an escape hatch - a dune rule that checks if a special file is present and use that as a version number if it exists when the package is built (or detect it if this file is not present - that would be what happens for most users). Would that work for your use case?

@jberdine @ceastlund I think that would also help you vendor ocamlformat in your organizations - let me know if that's not the case.

@gpetiot
Copy link
Collaborator

gpetiot commented Jan 22, 2021

Sorry if I fail to see the root of the issue, would having x.y.z+dev instead of x.y.z-commit-id fix the issue and be easier to implement with what is provided by dune-build-info?

edit: wait, in my distant memory I think they rely on the exact commit id at facebook, maybe we can just say that ocamlformat's version number is really undefined if it isn't a released version.

@gpetiot gpetiot requested a review from jberdine January 22, 2021 18:58
@gpetiot
Copy link
Collaborator

gpetiot commented Apr 14, 2021

The more I look at this issue, the more I see something that should get implemented by the build system, I'm closing this issue since the discussion didn't progress since it was opened!

@gpetiot gpetiot closed this Apr 14, 2021
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.

4 participants