Add minimal as a possible input for gap-version#83
Conversation
|
I think it is fair to use 4.10.2 and not 4.10.0, and so on. If someone truly has a package that works in the .1 but not .0, so be it -- the user should upgrade their GAp. We don't have the resources to be accurate on that level in our dependency declarations. |
| VERSION="" | ||
| IS_RELEASE=true | ||
|
|
||
| # We only check for releases in the official repository |
There was a problem hiding this comment.
Not for this PR, but perhaps an issue for a future date: this script is now quite long. Perhaps it should be extracted into its own file, which we can then better unit-test? (Or possibly it should be multiple fails).
While at it, I'd also consider whether this wouldn't be easier to read and debug if it was a python script (again: not asking for this in this PR, just an idea for future consideration)
There was a problem hiding this comment.
Extracting and adding unit tests would certainly be good to do in the future.
As for using Python, I personally find bash easier in situations like these (compatibility with multiple OS'es, dealing with input variables, calling external programs, etc)... but that's probably just me. I'm certainly not against such a change, if someone else is willing to implement it.
There was a problem hiding this comment.
thing is, bash is actually difficult to write cross platform as soon as one uses tools like sed, tar, etc. which differ between macOS and Linux. Parsing JSON or matching regex is much nicer and safer than in bash.
My biggest regret with the old ReleaseTools was that I wrote them in shell (not even bash, canb't assume that in general), and not in python.
That said, if it works, it works. I am not going to rewrite this code needlessly (and if I rewrite it, and not myself, but via AI...)
fingolfin
left a comment
There was a problem hiding this comment.
Great, thanks! Some nitpicks
Co-authored-by: Max Horn <max@quendi.de>
If
gap-versionis given the valueminimal, this will attempt to extract theDependencies.GAPvalue fromPackageInfo.gand use that as the GAP version to be built.If no version could be extracted from
PackageInfo.g, or if the extracted version is older than this action supports, we use the oldest version supported by this action instead.A small caveat: many packages declare something like "4.10" as minimal version, by which they probably mean "4.10.0". But by default this action expands minor versions to the newest one instead of the oldest, so "4.10" becomes "4.10.2" instead of "4.10.0".