Skip to content

Move to golangci v2#853

Merged
chombium merged 1 commit intocloudfoundry:mainfrom
jorbaum:golangciv2
Oct 2, 2025
Merged

Move to golangci v2#853
chombium merged 1 commit intocloudfoundry:mainfrom
jorbaum:golangciv2

Conversation

@jorbaum
Copy link
Copy Markdown
Contributor

@jorbaum jorbaum commented Sep 22, 2025

Description

Move to golangci-lint v2.

Issues were:

  • ST1005: Same error string capitalization for nicer logs
    • Also changed two asserts in a test
    • Also removed "Error:" prefix
  • QF1006: Lift break into loop condition
  • QF1008: Omit an embedded field
  • QF1003: Prefer tagged switch in some test files

Noteworthy changes in golangci-lint v2:

  • New config format (migrated with migrate command)
  • Default timeout is now 0 instead of 1 minute. See here for rationale. Kept ours unchanged for now.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing performed?

  • Unit tests
  • Integration tests
  • Acceptance tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation
  • I have added testing for my changes

@jorbaum jorbaum requested a review from a team as a code owner September 22, 2025 14:22
Noteworthy changes in golangci-lint v2:

* New config format (migrated with `migrate` command)
* Default timeout is now 0 instead of 1 minute.

Also fixes issues reported by golangci-lint v2:

* Same error string capitalization. Details:
    * Also changed two asserts in a test
    * Also removed "Error:" prefix
* Prefer embedded field
* Lift `break` into loop condition
* Prefer tagged switch in some test files
Copy link
Copy Markdown
Contributor

@chombium chombium left a comment

Choose a reason for hiding this comment

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

HI @jorbaum, thanks for the PR.

Please check the my comments.

sourceID: sourceID,
}
m.Tagged.tags = make(map[string]*loggregator_v2.Value)
m.tags = make(map[string]*loggregator_v2.Value)
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.

please revert this. The Tagged struct is used to unify the format of the tags among counters and gauges

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.

I've taken another look at this, This doesn't make any difference in the usage of the fields. As long as we keep and use the Tagged struct everywhere, there will be no problems.

sourceID: sourceID,
}
m.Tagged.tags = make(map[string]*loggregator_v2.Value)
m.tags = make(map[string]*loggregator_v2.Value)
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.

same as above

Copy link
Copy Markdown
Contributor

@chombium chombium left a comment

Choose a reason for hiding this comment

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

lgtm!

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group Oct 2, 2025
@chombium chombium merged commit 0618553 into cloudfoundry:main Oct 2, 2025
7 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants