Skip to content

chore: bump Go to 1.26.3 and centralize CLI build in integration tests#251

Open
erka wants to merge 1 commit into
open-feature:mainfrom
erka:go1-26
Open

chore: bump Go to 1.26.3 and centralize CLI build in integration tests#251
erka wants to merge 1 commit into
open-feature:mainfrom
erka:go1-26

Conversation

@erka
Copy link
Copy Markdown
Member

@erka erka commented May 20, 2026

This PR

  • Bump go to 1.26.3 and update GoBaseImage accordingly
  • Use go 1.25.0 for integration go test as lowest version for compatibility
  • Move CLI build out of individual tests into shared buildOpenFeatureCLI
  • Fix golangci-lint issues
  • Fix test/README.md and update new-generator.md

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the project to Go 1.26.3 and refactors string building across multiple generators to use fmt.Fprintf for improved efficiency. The integration test framework has been optimized to build the OpenFeature CLI container once and share it across all language-specific tests, rather than rebuilding it per test. Additionally, reflection logic in the manifest comparison was simplified using the Fields iterator. Feedback suggests extracting the hardcoded Go version used for compatibility testing in the Go integration suite into a centralized constant to improve maintainability.

Comment thread test/integration/cmd/go/run.go Outdated
Comment thread Makefile
@echo " ci - Run all CI checks locally (fmt, lint, test, verify-generate)"

# Tool versions
GOLANGCI_LINT_VERSION := v2.12.2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we configure Renovate to manage this version?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we switch to mise to manage our developer tools, then we would get the additional benefit of Renovate PRs without additional configuration, as Renovate has mise support.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

mise is nice, but we need the GC and TC support for adoption. It would be a bad idea to start without that agreement.

- Bump go to 1.26.3 and update GoBaseImage accordingly
- Use go 1.25.0 for integration go test as lowest version for compatibility
- Move CLI build out of individual tests into shared buildOpenFeatureCLI
- Bump golangci-lint to v2.12.2 and fix linter issues
- Fix test/README.md and update new-generator.md

Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
@erka
Copy link
Copy Markdown
Member Author

erka commented May 20, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the integration testing framework to build the CLI once and share it across tests, updates the project to Go 1.26.3, and optimizes string building using fmt.Fprintf. Feedback indicates several critical issues: the specified Go versions (1.25 and 1.26) and golangci-lint version (v2.12.2) are not valid released versions; the use of t.Fields() in the reflect package will cause compilation errors; and inconsistent indentation was found in the validation error format string.

Comment thread internal/manifest/compare.go
Comment thread go.mod
Comment thread test/integration/integration.go
Comment thread test/integration/integration.go
Comment thread Makefile
Comment thread internal/manifest/validate.go
@erka erka marked this pull request as ready for review May 20, 2026 16:50
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