Use Go version from go.mod in basic-checks #5154
Conversation
| @@ -77,7 +77,8 @@ METADATA_VAR += CommitSHA=$(EXTRA_VERSION) | |||
| METADATA_VAR += BaseDockerLabel=$(BASE_DOCKER_LABEL) | |||
| METADATA_VAR += DockerNamespace=$(DOCKER_NS) | |||
|
|
|||
| GO_VER = 1.24.0 | |||
| GO_VER := $(shell grep '^go[ \t]' < go.mod) | |||
There was a problem hiding this comment.
Note that this cannot use go list -m to obtain the version from the go directive since it will fail to parse the module file if the local Go version is behind the module's required Go version. This results in a less helpful error message from check_go_version.sh:
ERROR: is required to build Fabric and you are using 1.23.5. Please update go.
Using the approach implemented here, the error message is more helpful:
ERROR: 1.24.0 is required to build Fabric and you are using 1.23.5. Please update go.
|
I like the idea of specifying Go version in a single place (go.mod file). Do you think it would make sense for the github action workflows to reference the go.mod as well? |
| @@ -77,7 +77,8 @@ METADATA_VAR += CommitSHA=$(EXTRA_VERSION) | |||
| METADATA_VAR += BaseDockerLabel=$(BASE_DOCKER_LABEL) | |||
| METADATA_VAR += DockerNamespace=$(DOCKER_NS) | |||
|
|
|||
| GO_VER = 1.24.0 | |||
| GO_VER := $(shell grep '^go[ \t]' < go.mod) | |||
There was a problem hiding this comment.
For people newer to Go and scripting, it may be nice to add a comment like:
Go version maintained in the go.mod file.
The expected Go version was hard-coded in the Makefile, and this needed to be manually kept in sync with the CI and go.mod files. Since the go.mod entry needs to be accurate, this change uses the version from the go.mod file in the Makefile and no longer requires the Makefile to be manually kept up-to-date. Signed-off-by: Mark S. Lewis <[email protected]>
bestbeforetoday
force-pushed
the
makefile-go-version
branch
from
cd37296 to
d41073e
Compare
It seems like that should be possible. After checking out the code, either:
or:
Either approach could probably be rolled into a composite action (similar to the ones in fabric-samples) so it only appears as a single step in the existing workflows. |
I would probably lean towards the latter for consistency. Also the double setup-go may confuse people, not to mention it takes about 20 seconds each time. I'll merge this one for now. |
The expected Go version was hard-coded in the Makefile, and this needed to be manually kept in sync with the CI and go.mod files. Since the go.mod entry needs to be accurate, this change uses the version from the go.mod file in the Makefile and no longer requires the Makefile to be manually kept up-to-date.