Conversation
Add a config field go_version to allow the golang version to be specified. Update CI to test it.
Fix shellcheck.
Add description of the go_version configuration field.
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for specifying a Golang version via a new configuration field and updates the CI process to test the chosen version.
- Added a new "go_version" field in the configuration struct and updated validation logic
- Integrated the new field into template generation and error messages
- Updated the README to document the new configuration option
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/r10e-docker/r10edocker.go | Added go_version field, validation, and updated template usage |
| pkg/r10e-docker/nixpkgs_go_versions.go | Introduced mapping of Go versions to Nixpkgs commits and default version |
| README.md | Updated usage instructions to include go_version support |
Files not reviewed (2)
- pkg/r10e-docker/files/Dockerfile: Language not supported
- scripts/test_configs.sh: Language not supported
Comments suppressed due to low confidence (1)
pkg/r10e-docker/r10edocker.go:133
- [nitpick] Consider renaming the variable 'templateConfig' to avoid shadowing its type and to improve code clarity, for example, use 'tmplConfig' instead.
templateConfig := templateConfig{
Rename variable to address a GH copilot code review comment.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for a configurable Go version by introducing a "go_version" field in the configuration and updating the CI to use it. Key changes include:
- Adding a new "go_version" field to the Config struct and associated defaulting/validation logic.
- Introducing the mapping between Go versions and Nixpkgs commits in a new file.
- Updating documentation in the README to describe the new configuration field.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/r10e-docker/r10edocker.go | Adds go_version support with validation and updates template configuration |
| pkg/r10e-docker/nixpkgs_go_versions.go | Provides the mapping from Go versions to Nixpkgs commit data |
| README.md | Documents the new go_version field and its usage in the configuration file |
Files not reviewed (2)
- pkg/r10e-docker/files/Dockerfile: Language not supported
- scripts/test_configs.sh: Language not supported
Comments suppressed due to low confidence (1)
pkg/r10e-docker/r10edocker.go:160
- [nitpick] Consider renaming 'tmpltConfig' to a more descriptive name (e.g., 'templateConfigInstance') to improve clarity.
return template.Execute(f, tmpltConfig)
| } | ||
|
|
||
| nixPkgsCommitForGoVersion, ok := NixpkgsCommitForGoVersion[config.GoVersion] | ||
| if !ok { |
There was a problem hiding this comment.
The unsupported Go version check in GenR10eDocker duplicates validation already performed in ReadConfigFile. Consider consolidating the validation to reduce redundancy.
Add a config field go_version to allow the golang version to be specified. Update CI to test it.