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

Document and test supported versions of Go #205

Merged
merged 14 commits into from
Nov 28, 2024
Merged

Conversation

dmathieu
Copy link
Member

@dmathieu dmathieu commented Nov 27, 2024

Fixes #195

@dmathieu
Copy link
Member Author

I'm pondering adding a compatibility statement similar to the otel-go one.
https://github.com/open-telemetry/opentelemetry-go/tree/main?tab=readme-ov-file#compatibility

We don't really publish new releases with new Go versions though, and since this code is fully auto-generated, this may be a bit too much.

@dmathieu dmathieu marked this pull request as ready for review November 27, 2024 08:33
@dmathieu dmathieu requested a review from a team as a code owner November 27, 2024 08:33
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should have compatibility tests for all supported versions of Go (currently 1.22 and 1.23). These tests can be run on Ubuntu runner as there is no OS-specific code.

Right now there is nothing that checks if the generated code actually builds.

internal/tools/go.mod Outdated Show resolved Hide resolved
@dmathieu dmathieu changed the title Upgrade default go version in github workflow to 1.23 Document and test supported versions of Go Nov 27, 2024
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@dmathieu
Copy link
Member Author

Once this is merged, I will make the compatibility-test check required.

@pellared
Copy link
Member

Once this is merged, I will make the compatibility-test check required.

I think you can do it even before merging 😉

@dmathieu
Copy link
Member Author

It would prevent any other PR from being merged before this one.

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is wrong. go.sum files should not be empty.

@dmathieu
Copy link
Member Author

Their size has vastly reduced. They are not empty though.
Their content is retrieved from running go mod tidy.

@dmathieu dmathieu merged commit ad54459 into main Nov 28, 2024
5 checks passed
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.

Improper version of go in go.mod
3 participants