opentelemetry-go-contrib icon indicating copy to clipboard operation
opentelemetry-go-contrib copied to clipboard

build: add version check for go mod

Open flc1125 opened this issue 8 months ago • 5 comments

This seems to be skippable CHANGELOG.

Usage:

MINIMUM_GO_VERSION="go 1.23.0" make verify-goversion

flc1125 avatar Apr 15 '25 15:04 flc1125

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.0%. Comparing base (2adc680) to head (f4644ce).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7229   +/-   ##
=====================================
  Coverage   81.0%   81.0%           
=====================================
  Files        204     204           
  Lines      18080   18080           
=====================================
+ Hits       14651   14654    +3     
+ Misses      3003    3001    -2     
+ Partials     426     425    -1     

see 1 file with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Apr 15 '25 15:04 codecov[bot]

This is a nice idea. I would take a different approach though. In here, the go version is a constant within the test, which means an additional thing to change when we upgrade the Go version.

How about a new tool in internal/tools that does the same thing, with the expected Go version taken from an environment variable (or a CLI flag). Kind of like what we do here: https://github.com/open-telemetry/opentelemetry-go/tree/main/internal/tools/verifyreadmes

Then, we can have a new CI task that uses the proper Go version. We can add a MINIMUM_GO_VERSION in ci.yml. We'll have to update it on upgrades, but it's all in the same file.

dmathieu avatar Apr 16 '25 07:04 dmathieu

I will study how the internal/tools directory operates. I'll first set this PR to draft mode.

flc1125 avatar Apr 16 '25 10:04 flc1125

MINIMUM_GO_VERSION

@dmathieu I'm wondering if it's appropriate for us to use MINIMUM_GO_VERSION as the naming convention.

According to most people's understanding, MINIMUM_GO_VERSION should represent the minimum version we support. This means when we set it to v1.23.0, it actually allows versions like v1.23.1/v1.24.0 and so on.

However, the effect we actually want is for all Go Mods to be uniform.

For this reason, would EXPECTED_GO_VERSION be a more suitable choice?

If you have a better name, you can also let me know.

PS: Before finalizing this, I will tentatively submit it as MINIMUM_GO_VERSION for now.

flc1125 avatar Apr 23 '25 16:04 flc1125

EXPECTED_GO_VERSION seems like a good idea. Could you add tests to the Go file as well?

dmathieu avatar Apr 23 '25 19:04 dmathieu