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

Generate version.ml from satysfi.opam to make releasing easy #419

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

y-yu
Copy link
Contributor

@y-yu y-yu commented Oct 22, 2023

  • When releasing (for example Release SATySFi 0.0.10 #418), the releaser(= gfn-san) rewrite the version of src/frontend/main.ml by their hands
  • tools/genversion will generate such a version.ml 👇 when building
    let version = "0.0.9"
    and genversion gets version string ("0.0.9") from satisfy.opam using my ad-hoc regex
  • Then src/frontend/main.ml uses the generated version string
  • I think that this PR reduces a step of releasing process 🤔

@y-yu y-yu force-pushed the generate-version-from-opam-file branch from a678281 to 21c4764 Compare October 23, 2023 01:57
@na4zagin3
Copy link
Contributor

na4zagin3 commented Oct 23, 2023

How about having OPAM run substitute commands like na4zagin3/satyrographos#189 (dune subst and sed)?

@y-yu
Copy link
Contributor Author

y-yu commented Oct 23, 2023

Thanks for the comment 😄 I saw the technique of na4zagin3/satyrographos#189.
First of all, the best choice is what gfn-san likes, I think. Based on that, my opnions about this PR (using OCaml way) v.s. dune and sed(zagin-san's way) are:

  • zagin-san's way is simpler than my PR way
  • In sed, there are some differences between BSD and GNU so I don't like to use sed😇
    • Indeed, on this usecase, sed does simple replacement only so is it no matter? 🤔
    • OCaml is more complecate than sed but it's the same behavior on BSD and GNU
  • OS commands and shell scripts are not unit testable

@y-yu
Copy link
Contributor Author

y-yu commented Oct 23, 2023

Even if we'll choice either way, it would be better to write the version info to the other file (such like version.opam) rather than using single OPAM file 🤔 in order to manipulate it by GitHub Actions(automatic release scripts) or so on. (but it's not possible on the current OPAM)
It may be a lesson for SATySFi 0.1.0 package manager setting file 👀

@gfngfn
Copy link
Owner

gfngfn commented Feb 5, 2024

I'm so sorry for the late response.

I can't confidently tell which of the two approaches is better, but at least I like Yoshimura-san's OS-independent implementation. I would thus like to merge this PR for the moment.

Thank you for the improvement!

@gfngfn gfngfn merged commit e1e2ad0 into gfngfn:master Feb 5, 2024
@y-yu y-yu deleted the generate-version-from-opam-file branch February 5, 2024 13:44
@na4zagin3
Copy link
Contributor

I'm so sorry for my long silence. Avoiding using sed achieves OS-independency obviously, which is nice! This is too late but anyway I retract my comment. Thank you!

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.

3 participants