Skip to content

Conversation

@timja
Copy link
Member

@timja timja commented Jan 1, 2025

Fixes #2

@timja timja marked this pull request as ready for review January 2, 2025 10:27
@timja timja requested a review from abayer January 2, 2025 10:28
cobraCmd := &cobra.Command{
Use: "fetch",
Short: "Fetch raw usage data from Azure",
Run: func(cmd *cobra.Command, args []string) {
Copy link
Member Author

Choose a reason for hiding this comment

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

not 100% sure on if this is a great idea, its much clearer what the args could be used with the old name. Although not too keen on disabling the rule

Copy link
Contributor

@lemeurherve lemeurherve Sep 25, 2025

Choose a reason for hiding this comment

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

Current golangci-lint version in the agents is the 2.5.0: https://github.com/jenkins-infra/packer-images/blob/c4df4c868e156f1b3d31f9d0d4f15bd5e16a81c6/provisioning/tools-versions.yml#L17

It requires a migration (https://golangci-lint.run/docs/product/migration-guide/) to get make lint running.

$ make lint
--> Running golangci-lint
golangci-lint --version
golangci-lint has version 2.5.0 built with go1.25.1 from ff63786 on 2025-09-21T18:53:13Z
golangci-lint run
Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
make: *** [lint] Error 3
$ golangci-lint migrate
jsonschema: "run" does not validate with "/properties/run/additionalProperties": additional properties 'deadline' not allowed
jsonschema: "linters" does not validate with "/properties/linters/additionalProperties": additional properties 'include' not allowed
The command is terminated due to an error: the configuration contains invalid elements

Running the migrate command with --skip-validation result in an almost completely new config file, but I don't know if we're loosing something or not on the way.

version: "2"
linters:
  enable:
    - goconst
    - gosec
    - misspell
    - revive
  exclusions:
    generated: lax
    rules:
      - path: (.+)\.go$
        text: 'package-comments: should have a package comment'
      - path: (.+)\.go$
        text: G115
      - path: (.+)\.go$
        text: SA1019
    paths:
      - third_party$
      - builtin$
      - examples$
formatters:
  enable:
    - gofmt
    - goimports
  exclusions:
    generated: lax
    paths:
      - third_party$
      - builtin$
      - examples$

With that resulting config, here are the warnings returned with make lint:

$ make lint
--> Running golangci-lint
golangci-lint --version
golangci-lint has version 2.5.0 built with go1.25.1 from ff63786 on 2025-09-21T18:53:13Z
golangci-lint run
cmd/jenkins-usage-stats/fetch.go:87:26: QF1008: could remove embedded field "ContainerListBlobFlatSegmentResult" from selector (staticcheck)
                for _, v := range resp.ContainerListBlobFlatSegmentResult.Segment.BlobItems {
                                       ^
db.go:430:5: QF1009: probably want to use time.Time.Equal instead (staticcheck)
        if prevReport.ReportTime == ts || ts.Before(prevReport.ReportTime) {
           ^
report.go:1498:6: QF1001: could apply De Morgan's law (staticcheck)
                if !(y == currentYear && m == currentMonth) {
                   ^
report.go:1664:6: QF1001: could apply De Morgan's law (staticcheck)
                if !(y == currentYear && m == currentMonth) {
                   ^
4 issues:
* staticcheck: 4

Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise make build and make test pass locally, the rest of your PR LGTM.

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.

Set up CI

2 participants