-
Notifications
You must be signed in to change notification settings - Fork 1
feat: update go linter in CI #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| // acceptance testing. The factory function will be invoked for every Terraform | ||
| // CLI command executed to create a provider server to which the CLI can | ||
| // reattach. | ||
| var testAccProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServer, error){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will add all this test stuff back in in my other PR, but since it's unused here, we are getting that lint warning.
| "github.com/hashicorp/terraform-plugin-framework/types" | ||
|
|
||
| // "github.com/hashicorp/terraform-plugin-log/tflog" | ||
| // "github.com/hashicorp/terraform-plugin-log/tflog". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rek what is the . for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ golangci-lint run
internal/service/folders/data_source.go:10:54: Comment should end in a period (godot)
// "github.com/hashicorp/terraform-plugin-log/tflog"
^It fixes this issue. But perhaps I should just remove that line instead.
| return []ClickUpTeamMemberDataSourceModel{} | ||
| } | ||
| result := make([]ClickUpTeamMemberDataSourceModel, len(members), len(members)) | ||
| result := make([]ClickUpTeamMemberDataSourceModel, 0, len(members)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rek I think it makes sense to just use make([]ClickUpTeamMemberDataSourceModel, len(members)) here and leave the capacity off it'll just get set the same automatically by make.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is trying to resolve this issue from golangci-lint run:
internal/service/teams/data_source.go:93:53: S1019: should use make([]ClickUpTeamMemberDataSourceModel, len(members)) instead (gosimple)
result := make([]ClickUpTeamMemberDataSourceModel, len(members), len(members))
The problem with the gosimple suggestion is that it causes this further down the line:
internal/service/teams/data_source.go:117:12: append to slice `result` with non-zero initialized length (makezero)
result = append(result, mem)
Here is the generated explanation:
there is a potential issue related to the use of
makeandappendtogether, which could lead to unexpected results.The line
result := make([]ClickUpTeamMemberDataSourceModel, len(members))creates a slice ofClickUpTeamMemberDataSourceModelwith a length equal tolen(members). This means that the sliceresultinitially containslen(members)zero values.Then, in the loop, you're using
appendto add elements toresult. Theappendfunction does not replace the zero values, but adds new elements to the end of the slice. This means that after the loop,resultwill containlen(members)zero values, followed by the actual data.
I have changed that part now too, have a look and see what you think.
Since my other branch is getting quite big, I thought I would split out some smaller PR's.
Here is one to make the lint go green. (it's mostly just running the auto-linter that CI uses locally)
Also: