Cleanup: uplevel our technical hygiene by enabling additional Golang linters #5899

bendory · 2022-12-20T16:42:45Z

bendory · 2022-12-20T16:43:24Z

I would assign this issue to me, but I don't seem to have the authority to do so. 🙃

bendory · 2022-12-20T16:44:58Z

#5871 Added govet to our Golang linter suite
#5881 Replaced deprecated deadcode linter with unused linter
#5889 Enabled the whitespace linter.

vdemeester · 2022-12-21T11:41:29Z

/assign @bendory
(btw, if you comment /assign, it should automatically assign the issue to you 😉 )

vdemeester · 2022-12-21T11:41:48Z

(ah lol, @jerop manually assigned you already 😂 )

bendory · 2022-12-22T14:09:50Z

See also: tektoncd/community#908 Add Go style guide to community standards (thanks @lbernick)

There are no expected functional changes in this PR. The code changes are duplicate words discovered by the `dupword` linter, for example, `the the` --> `the`. Occasional deduped words appear in unit test strings where I've uniqified the words in question. The `dupword` linter will be enabled in #5918. Context: #5899 /kind cleanup

This change will update our plumbing to include #1308 as well as any other changes since the last upgrade. Context: tektoncd/pipeline#5918 (comment) Issue: tektoncd/pipeline#5899

Given cleanup in #5923, there are no required code changes for this linter to lint clean. Context: #5899 /kind cleanup

Enabled `bodyclose`, `decorder`, and `typecheck`. These currently lint clean, so there are no needed code changes. Context: #5899 /kind cleanup

This linter cleans up our test output by omiting helper functions from test failure messages, instead labeling the failure with the calling function's details. This makes it easier to locate the true origin of the failure. There are no expected functional changes in this PR. The code changes in this PR are all changes to testing code to comply with Go style and best practices, specifically `t.Helper`: - https://pkg.go.dev/testing#T.Helper - https://goo.gle/go-style/best-practices#error-handling-in-test-helpers Context: #5899 /kind cleanup

This PR enables golangci presets format, import, metalinter, module, unused, test. Code changes correspond to the issues found by linters. There are no expected functional changes in this PR. Context: #5899 /kind cleanup - Submitter Checklist As the author of this PR, please check off the items in this checklist: - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [N/A] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [N/A] Release notes contains the string "action required" if the change requires additional action from users switching to the new release - Release Notes ```release-note NONE ```

This PR opts us into remaining `golangci` presets and removes the `disable-all` configuration such that we automatically inherit new default linters. To get here, I opted out from `staticcheck` and `ineffassign`, both of which will require us to clean up existing issues prior to enablement. That can be done in future PRs. Context: #5899

Enabled: - containedctx - dogsled - duple - errchkjson - errorlint - errname - exhaustive - goconst - ireturn - staticcheck - unparam - wrapcheck There are no expected functional changes in this PR. Context: tektoncd/pipeline#5899 /kind cleanup - Submitter Checklist - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [N/A] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [N/A] Release notes contains the string "action required" if the change requires additional action from users switching to the new release ```release-note NONE ```

- Sort linters list. - Replaces deprecated `exclude` configuration for `errcheck` with replacement `exclude-functions` configuration. - Deletes obsolete `depguard` configuration. - Deletes obsolete skip `unused` configuration for `pipelinerun_test.go`. Context: tektoncd#5899 /kind cleanup  - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [x] Release notes contains the string "action required" if the change requires additional action from users switching to the new release ```release-note NONE ```

There are no expected functional changes in this PR. Made nominal code changes to adhere to linter standards, specifically more systematic use of `pipeline.*RunControllerName` constants. Context: tektoncd#5899 /kind cleanup - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [x] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

This linter identifies unused variable assignments. - Fixes identified lint issues. - Updated configuration to ignore inefective assignments in unit tests. Context: tektoncd#5899 /kind cleanup  - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [x] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Enables `containedctx` linter to identify use of antipattern use of context as a struct field. Go style is to pass the context directly instead. To get here, I added exclusions for the existing detected use; enabling the linter will prevent additional use of the antipattern. There are no expected functional changes in this PR. Context: tektoncd#5899 /kind cleanup  - Submitter Checklist - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [x] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

There are no expected functional changes in this PR. Made non-functional code changes to adhere to linter standards. Context: #5899 See related style docs: - https://github.com/golang/go/wiki/Errors#naming - https://google.github.io/styleguide/go/guide#concision - https://go.dev/blog/errors-are-values

There are no expected functional changes in this PR. Made non-functional code changes to adhere to linter standards. Context: tektoncd#5899 See related style docs: - https://github.com/golang/go/wiki/Errors#naming - https://google.github.io/styleguide/go/guide#concision - https://go.dev/blog/errors-are-values

There are no expected functional changes in this PR. Made non-functional code changes to adhere to Golang error standards. Context: #5899 See related style docs: - https://pkg.go.dev/errors - https://go.dev/blog/errors-are-values /kind cleanup - [N/A] Has [Docs](https://github.com/tektoncd/community/blob/main/standards.md#docs) included if any changes are user facing - [N/A] Has [Tests](https://github.com/tektoncd/community/blob/main/standards.md#tests) included if any functionality added or changed - [x] Follows the [commit message standard](https://github.com/tektoncd/community/blob/main/standards.md#commits) - [x] Meets the [Tekton contributor standards](https://github.com/tektoncd/community/blob/main/standards.md) (including functionality, content, code) - [x] Has a kind label. You can add one by adding a comment on this PR that contains `/kind <type>`. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [N/A] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings) - [N/A] Release notes contains the string "action required" if the change requires additional action from users switching to the new release ```release-note NONE ```

- Addressed newly identified `errorlint` items. - Narrowed `nolint` --> `nolint:gocritic`. - Removed 2 `nolint` comments by addressing identified issues. - Also: cleaned up some test logic in pipelinerun_test.go. There are no expected functional changes in this PR. Context: #5899 /kind cleanup

The [exhaustive](https://pkg.go.dev/github.com/nishanths/exhaustive) linter checks exhaustiveness of switch statements, ensuring completeness of case statements. There are no expected functional changes in this CL. Context: #5899

This enables some new linters and disables some deprecated ones. Context: tektoncd/pipeline#5899 This PR is step one in unblocking tektoncd/pipeline#6518 /kind cleanup

This change will update our plumbing to include #1389 as well as any other changes since the last upgrade. Context: tektoncd/pipeline#6518 Issue: tektoncd/pipeline#5899 /kind cleanup

See this thread: https://tektoncd.slack.com/archives/CJ4ERJWAU/p1681920119203019 Context: #5899 Disabled `revive` linter; upgrade results in a high number of new unused variables not previously identified in the previous version.

tekton-robot · 2023-07-02T17:28:06Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

tekton-robot · 2023-08-01T18:13:04Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

tekton-robot · 2023-08-31T18:19:19Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

tekton-robot · 2023-08-31T18:19:21Z

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bendory added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 20, 2022

bendory mentioned this issue Dec 20, 2022

Sorted linters -- because neatness counts. #5898

Merged

3 tasks

jerop assigned bendory Dec 20, 2022

This was referenced Dec 20, 2022

Added documentation for local golangci-lint use. #5900

Merged

Enabled gosimple linter. #5904

Merged

This was referenced Dec 28, 2022

Enabled thelper linter. #5927

Merged

Upgraded test-runner image to latest. tektoncd/plumbing#1309

Merged

This was referenced Dec 28, 2022

Cleanup: fix failures from updated golangci-lint. #5928

Merged

Cleaned up golangci-lint and added additional linters. tektoncd/chains#661

Merged

tekton-robot pushed a commit that referenced this issue Dec 29, 2022

Enabled dupword and usestdlibvars linters.

8d8a9d1

Given cleanup in #5923, there are no required code changes for this linter to lint clean. Context: #5899 /kind cleanup

tekton-robot pushed a commit that referenced this issue Dec 29, 2022

Enabled additional linters.

1920789

Enabled `bodyclose`, `decorder`, and `typecheck`. These currently lint clean, so there are no needed code changes. Context: #5899 /kind cleanup

bendory mentioned this issue Jan 3, 2023

Update golangci-lint in Makefile and tools/go.mod to v1.50.1 #5935

Merged

7 tasks

lbernick added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Feb 7, 2023

bendory mentioned this issue Feb 27, 2023

Hygiene: enable additional linter presets. #6236

Merged

3 tasks

bendory mentioned this issue Feb 28, 2023

Hygiene: enable additional lint presets. #6247

Merged

5 tasks

bendory mentioned this issue Mar 9, 2023

Hygiene: enable errorlint. #6340

Merged

3 tasks

This was referenced Mar 31, 2023

Cleanup: addressed various nolint items. 🧹 🧹 🧹 #6477

Merged

Hygiene: enable exhaustive linter. #6484

Merged

This was referenced Apr 10, 2023

Hygiene: enabled presets and various linters. #6518

Merged

Upgraded golangci-lint to 1.52.2 (latest). tektoncd/plumbing#1389

Merged

bendory mentioned this issue Apr 17, 2023

Upgraded test-runner image to latest. tektoncd/plumbing#1396

Merged

1 task

This was referenced Apr 18, 2023

Hygiene: enabled presets properly. tektoncd/chains#777

Merged

Use the system-installed version of golangci-lint. #6560

Merged

bendory mentioned this issue Apr 24, 2023

Hygiene: Enabled new linters and increased timeout. tektoncd/chains#781

Merged

3 tasks

tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 2, 2023

tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 1, 2023

tekton-robot closed this as completed Aug 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup: uplevel our technical hygiene by enabling additional Golang linters #5899

Cleanup: uplevel our technical hygiene by enabling additional Golang linters #5899

bendory commented Dec 20, 2022 •

edited

Loading

bendory commented Dec 20, 2022

bendory commented Dec 20, 2022 •

edited

Loading

vdemeester commented Dec 21, 2022

vdemeester commented Dec 21, 2022

bendory commented Dec 22, 2022 •

edited

Loading

tekton-robot commented Jul 2, 2023

tekton-robot commented Aug 1, 2023

tekton-robot commented Aug 31, 2023

tekton-robot commented Aug 31, 2023

Cleanup: uplevel our technical hygiene by enabling additional Golang linters #5899

Cleanup: uplevel our technical hygiene by enabling additional Golang linters #5899

Comments

bendory commented Dec 20, 2022 • edited Loading

Feature request

bendory commented Dec 20, 2022

bendory commented Dec 20, 2022 • edited Loading

vdemeester commented Dec 21, 2022

vdemeester commented Dec 21, 2022

bendory commented Dec 22, 2022 • edited Loading

tekton-robot commented Jul 2, 2023

tekton-robot commented Aug 1, 2023

tekton-robot commented Aug 31, 2023

tekton-robot commented Aug 31, 2023

bendory commented Dec 20, 2022 •

edited

Loading

bendory commented Dec 20, 2022 •

edited

Loading

bendory commented Dec 22, 2022 •

edited

Loading