Skip to content

Update build tooling from starter template and fix linter issues#964

Open
hanzei wants to merge 1 commit intomasterfrom
update-from-starter-template
Open

Update build tooling from starter template and fix linter issues#964
hanzei wants to merge 1 commit intomasterfrom
update-from-starter-template

Conversation

@hanzei
Copy link
Contributor

@hanzei hanzei commented Feb 4, 2026

Summary

  • Update Go version from 1.24.6 to 1.24.11
  • Update golangci-lint to v2.8.0 with new config format (version 2)
  • Update gotestsum to v1.13.0
  • Update Node.js version from 16.13.1 to 20.11
  • Add manifest-check target to Makefile
  • Fix all linter issues to pass make check-style

Changes

Build Tooling

  • Updated .golangci.yml to version 2 format with new linters (bidichk, makezero, modernize, unqueryvet)
  • Updated Makefile with new tool versions and manifest-check target
  • Removed duplicate GO_BUILD_FLAGS from build/setup.mk
  • Added "check" command to build/manifest/main.go

Code Modernization

  • Replaced interface{} with any throughout codebase
  • Replaced custom contains/SliceContainsString functions with slices.Contains
  • Used Go 1.24 features (strings.SplitSeq, fmt.Appendf)
  • Converted if-else chains to tagged switches (QF1003)
  • Simplified embedded field selectors (QF1008: c.Context.Ctxc.Ctx)
  • Used strings.Builder for efficient string concatenation in loops
  • Applied De Morgan's law simplifications (QF1001)
  • Fixed directory permissions (gosec G301: 0755 → 0750)
  • Added nolint comments for deprecated cipher functions (SA1019)

Config Files

  • Updated .nvmrc from 16.13.1 to 20.11
  • Fixed Makefile pattern syntax in .editorconfig
  • Cleaned up duplicate entries in .gitignore

Test plan

  • make check-style passes with 0 issues
  • make test passes (435 Go tests + 22 webapp tests)

🤖 Generated with Claude Code

- Update Go version from 1.24.6 to 1.24.11
- Update golangci-lint to v2.8.0 with new config format
- Update gotestsum to v1.13.0
- Update Node.js version from 16.13.1 to 20.11
- Add manifest-check target to Makefile
- Use Go 1.24 features (strings.SplitSeq, slices.Contains)
- Replace interface{} with any throughout codebase
- Replace custom contains functions with slices.Contains
- Convert if-else chains to tagged switches
- Simplify embedded field selectors (c.Context.Ctx → c.Ctx)
- Use strings.Builder for efficient string concatenation
- Apply De Morgan's law simplifications
- Fix directory permissions (0755 → 0750)
- Add license headers to build files

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@hanzei hanzei requested a review from a team as a code owner February 4, 2026 13:31
Copy link

@nevyangelova nevyangelova left a comment

Choose a reason for hiding this comment

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

Thanks @hanzei some minor changes. Also I see tests for contains and SliceContainsString were removed but the tests for containsValue in utils_test.go were also removed. Consider keeping at least one integration test to verify the behavior with your specific use cases.

// Only send down fields to client that are needed
type RepositoryResponse struct {
DefaultRepo RepoResponse `json:"defaultRepo,omitempty"`
DefaultRepo RepoResponse `json:"defaultRepo"`

Choose a reason for hiding this comment

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

Is there a reason for removing omitempty? it causes an empty {} to be sent when no default repo is set. The frontend checks if (yourReposByOrg?.defaultRepo) which is truthy for {} causing undefined values to be passed to onChangeForRepo().


orgs := strings.Split(configuredOrgs, ",")
for _, org := range orgs {
for org := range strings.SplitSeq(configuredOrgs, ",") {

Choose a reason for hiding this comment

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

Those are Go 1.24+ features. we should ensure all CI/CD pipelines and developer environments support them

@@ -1635,7 +1600,6 @@ func TestHandleSubscribe(t *testing.T) {
name: "default case, handleSubscribesAdd called",
parameters: []string{"invalid_parameter_1", "invalid_parameter_2", "invalid_parameter_3"},
setup: func() {

Choose a reason for hiding this comment

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

is this empty function needed?

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.

2 participants