Enable `gocritic` and don't ignore globally #3159

qwerty287 · 2024-01-10T11:14:02Z

Use nolint directives instead.

From #2960

xoxys · 2024-01-10T11:17:04Z

Instead of adding nolint 100 times I would keep global exclude rules for files that are expect to be excluded.

woodpecker-bot · 2024-01-10T11:22:05Z

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-3159.surge.sh

qwerty287 · 2024-01-10T11:25:28Z

Instead of adding nolint 100 times I would keep global exclude rules for files that are expect to be excluded.

Many of these exclusions were not about whole files.
Some are false-positives.
The only linter which is somewhat responsible for more issues if forbidigo. You're right that these add a lot of these directives, but actually, not all of them are required. I'll try to refactor this a bit more to fix this.

codecov · 2024-01-10T11:29:12Z

Codecov Report

Attention: 80 lines in your changes are missing coverage. Please review.

Comparison is base (f342071) 34.91% compared to head (bc590f2) 34.93%.

Files	Patch %	Lines
cmd/server/server.go	0.00%	29 Missing ⚠️
cmd/server/setup.go	0.00%	14 Missing ⚠️
server/plugins/encryption/encryption_builder.go	0.00%	8 Missing ⚠️
server/web/web.go	58.33%	4 Missing and 1 partial ⚠️
pipeline/frontend/yaml/parse.go	0.00%	4 Missing ⚠️
cmd/agent/agent.go	0.00%	3 Missing ⚠️
server/api/user.go	0.00%	3 Missing ⚠️
server/forge/gitlab/convert.go	25.00%	3 Missing ⚠️
server/plugins/encryption/tink_keyset_watcher.go	0.00%	3 Missing ⚠️
server/store/datastore/migration/common.go	40.00%	2 Missing and 1 partial ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3159      +/-   ##
==========================================
+ Coverage   34.91%   34.93%   +0.01%     
==========================================
  Files         228      228              
  Lines       14789    14795       +6     
==========================================
+ Hits         5164     5168       +4     
- Misses       9246     9251       +5     
+ Partials      379      376       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

qwerty287 · 2024-01-10T13:09:56Z

@xoxys Can you re-review with bb6af67?

Instead of using log.Fatal, the error should be returned.

xoxys · 2024-01-10T13:16:42Z

I'm not sure if this has unwanted side effects. If, e.g. the host can't be parsed or is not set, the server should be terminated instead of just returning the error, no?

xoxys · 2024-01-10T13:19:00Z

Ah, I guess this is covered by the entrypoint e.g. bb6af67#diff-53cd25fc908d5e63cc8675afbd4ff0910572a985a7cfec27f4460eb674f3b98eR46-R47 now, and we just need one Fatal linting exception per package?

qwerty287 · 2024-01-10T13:31:43Z

we just need one Fatal linting exception per package?

Exactly. (Exception: https://github.com/woodpecker-ci/woodpecker/pull/3159/files#diff-dfc28d1b6c36f02b0ebf9bc16c5c7fbca8c60202f74b61a13298ef3b35e12e73 for example is used in a goroutine, I don't know if there maybe is a better way, but secrets encryption is not active anyways.)

xoxys

LGTM, but you might want to wait for a second review.

This PR was opened by the [ready-release-go](https://github.com/woodpecker-ci/plugin-ready-release-go) plugin. When you're ready to do a release, you can merge this pull-request and a new release with version `2.2.0` will be created automatically. If you're not ready to do a release yet, that's fine, whenever you add more changes to `main` this pull-request will be updated. ## Options - [ ] Mark this version as a release candidate ## [2.2.0](https://github.com/woodpecker-ci/woodpecker/releases/tag/2.2.0) - 2024-01-21 ### 🔒 Security - Update web dependencies [[#3234](#3234)] ### ✨ Features - Support custom steps entrypoint [[#2985](#2985)] ### 📚 Documentation - Add 2.2 docs [[#3237](#3237)] - Fix/improve issue templates [[#3232](#3232)] - Delete `FUNDING.yaml` [[#3193](#3193)] - Remove contributing/security to use globally defined [[#3192](#3192)] - Add "Kaniko" Plugin [[#3183](#3183)] - Document core development ideas [[#3184](#3184)] - Add continous deployment cookbook [[#3098](#3098)] - Make k8s backend configuration docs in the same format as others [[#3081](#3081)] - Hide backend config options from TOC [[#3126](#3126)] - Add X/Twitter account [[#3127](#3127)] - Add ansible plugin [[#3115](#3115)] - Format depends_on example [[#3118](#3118)] - Use WOODPECKER_AGENT_SECRET instead of deprecated alternative [[#3103](#3103)] - Add Reviewdog ESLint plugin [[#3102](#3102)] - Mark local backend as stable [[#3088](#3088)] - Update Owners 2024 [[#3075](#3075)] - Add reviewdog golangci plugin [[#3080](#3080)] - Add Codeberg Pages Deploy plugin to plugins list [[#3054](#3054)] ### 🐛 Bug Fixes - Fixed Pods creation of WP services [[#3236](#3236)] - Fix Bitbucket get pull requests that ignores pagination [[#3235](#3235)] - Make PipelineConfig unique again [[#3215](#3215)] - Fix feed sorting [[#3155](#3155)] - Step status update dont set to running again once it got stoped [[#3151](#3151)] - Use step uuid instead of name in GRPC status calls [[#3143](#3143)] - Use UUID instead of step name where possible [[#3136](#3136)] - Use step type to detect services in Kubernetes backend [[#3141](#3141)] - Fix config base64 parsing to utf-8 [[#3110](#3110)] - Pin Gitea version [[#3104](#3104)] - Fix step `depends_on` as string in schema [[#3099](#3099)] - Fix slice unmarshaling [[#3097](#3097)] - Allow PR secrets to be used on close [[#3084](#3084)] - make event in pipeline schema also a constraint_list [[#3082](#3082)] - Fix badge's repoUrl with rootpath [[#3076](#3076)] - Load changed files for closed PR [[#3067](#3067)] - Fix build output paths [[#3065](#3065)] - Fix `when` and `depends_on` [[#3063](#3063)] - Fix DAG cycle detection [[#3049](#3049)] - Fix duplicated icons [[#3045](#3045)] ### 📈 Enhancement - Retrieve all user repo perms with a single API call [[#3211](#3211)] - Secured kubernetes backend configuration [[#3204](#3204)] - Use `assert` for tests [[#3201](#3201)] - Replace `goimports` with `gci` [[#3202](#3202)] - Remove multipart logger [[#3200](#3200)] - Added protocol in port configuration [[#2993](#2993)] - Kubernetes AppArmor and seccomp [[#3123](#3123)] - `cli exec`: let override existing environment values but print a warning [[#3140](#3140)] - Enable golangci linter forcetypeassert [[#3168](#3168)] - Enable golangci linter contextcheck [[#3170](#3170)] - Remove panic recovering [[#3162](#3162)] - More docker backend test remove more undocumented [[#3156](#3156)] - Lowercase all log strings [[#3173](#3173)] - Cleanups + prefer .yaml [[#3069](#3069)] - Use UUID as podName and cleanup arguments for Kubernetes backend [[#3135](#3135)] - Enable golangci linter stylecheck [[#3167](#3167)] - Clean up logging [[#3161](#3161)] - Enable `gocritic` and don't ignore globally [[#3159](#3159)] - Remove steps for publishing release branches [[#3125](#3125)] - Enable `nolintlint` [[#3158](#3158)] - Enable some linters [[#3129](#3129)] - Use name in backend types instead of alias [[#3142](#3142)] - Make service icon rotate [[#3149](#3149)] - Add step name as label to docker containers [[#3137](#3137)] - Use js-base64 on pipeline log page [[#3146](#3146)] - Flexible image pull secret reference [[#3016](#3016)] - Always show pipeline step list [[#3114](#3114)] - Add loading spinner and no pull request text [[#3113](#3113)] - Fix timeout settings contrast [[#3112](#3112)] - Unfold workflow when opening via URL [[#3106](#3106)] - Remove env argument of addons [[#3100](#3100)] - Move `cmd/common` to `shared` [[#3092](#3092)] - use semver for version comparsion [[#3042](#3042)] - Extend create plugin docs [[#3062](#3062)] - Remove old files [[#3077](#3077)] - Indicate if step is service [[#3078](#3078)] - Add imports checks to linter [[#3056](#3056)] - Remove workflow version again [[#3052](#3052)] - Add option to disable version check in admin web UI [[#3040](#3040)] ### Misc - chore(deps): update docker.io/woodpeckerci/plugin-docker-buildx docker tag to v3 [[#3229](#3229)] - Docs: Fix expression syntax docs url [[#3208](#3208)] - Add schema test for depends_on [[#3205](#3205)] - chore(deps): lock file maintenance [[#3190](#3190)] - Do not run prettier with pre-commit [[#3196](#3196)] - fix(deps): update module github.com/google/go-github/v57 to v58 [[#3187](#3187)] - chore(deps): update docker.io/golang docker tag to v1.21.6 [[#3189](#3189)] - chore(deps): update docker.io/woodpeckerci/plugin-docker-buildx [[#3186](#3186)] - fix(deps): update golang (packages) [[#3185](#3185)] - declare different when statements once and reuse them [[#3176](#3176)] - Add `make clean-all` [[#3152](#3152)] - Fix `version.json` updates [[#3057](#3057)] - [pre-commit.ci] pre-commit autoupdate [[#3101](#3101)] - Update dependency @vitejs/plugin-vue to v5 [[#3074](#3074)] - Use CI vars for plugin [[#3061](#3061)] - Use `yamllint` [[#3066](#3066)] - Use dag in ci config [[#3010](#3010)]

Use `nolint` directives instead. From woodpecker-ci#2960

This PR was opened by the [ready-release-go](https://github.com/woodpecker-ci/plugin-ready-release-go) plugin. When you're ready to do a release, you can merge this pull-request and a new release with version `2.2.0` will be created automatically. If you're not ready to do a release yet, that's fine, whenever you add more changes to `main` this pull-request will be updated. ## Options - [ ] Mark this version as a release candidate ## [2.2.0](https://github.com/woodpecker-ci/woodpecker/releases/tag/2.2.0) - 2024-01-21 ### 🔒 Security - Update web dependencies [[woodpecker-ci#3234](woodpecker-ci#3234)] ### ✨ Features - Support custom steps entrypoint [[woodpecker-ci#2985](woodpecker-ci#2985)] ### 📚 Documentation - Add 2.2 docs [[woodpecker-ci#3237](woodpecker-ci#3237)] - Fix/improve issue templates [[woodpecker-ci#3232](woodpecker-ci#3232)] - Delete `FUNDING.yaml` [[woodpecker-ci#3193](woodpecker-ci#3193)] - Remove contributing/security to use globally defined [[woodpecker-ci#3192](woodpecker-ci#3192)] - Add "Kaniko" Plugin [[woodpecker-ci#3183](woodpecker-ci#3183)] - Document core development ideas [[woodpecker-ci#3184](woodpecker-ci#3184)] - Add continous deployment cookbook [[woodpecker-ci#3098](woodpecker-ci#3098)] - Make k8s backend configuration docs in the same format as others [[woodpecker-ci#3081](woodpecker-ci#3081)] - Hide backend config options from TOC [[woodpecker-ci#3126](woodpecker-ci#3126)] - Add X/Twitter account [[woodpecker-ci#3127](woodpecker-ci#3127)] - Add ansible plugin [[woodpecker-ci#3115](woodpecker-ci#3115)] - Format depends_on example [[woodpecker-ci#3118](woodpecker-ci#3118)] - Use WOODPECKER_AGENT_SECRET instead of deprecated alternative [[woodpecker-ci#3103](woodpecker-ci#3103)] - Add Reviewdog ESLint plugin [[woodpecker-ci#3102](woodpecker-ci#3102)] - Mark local backend as stable [[woodpecker-ci#3088](woodpecker-ci#3088)] - Update Owners 2024 [[woodpecker-ci#3075](woodpecker-ci#3075)] - Add reviewdog golangci plugin [[woodpecker-ci#3080](woodpecker-ci#3080)] - Add Codeberg Pages Deploy plugin to plugins list [[woodpecker-ci#3054](woodpecker-ci#3054)] ### 🐛 Bug Fixes - Fixed Pods creation of WP services [[woodpecker-ci#3236](woodpecker-ci#3236)] - Fix Bitbucket get pull requests that ignores pagination [[woodpecker-ci#3235](woodpecker-ci#3235)] - Make PipelineConfig unique again [[woodpecker-ci#3215](woodpecker-ci#3215)] - Fix feed sorting [[woodpecker-ci#3155](woodpecker-ci#3155)] - Step status update dont set to running again once it got stoped [[woodpecker-ci#3151](woodpecker-ci#3151)] - Use step uuid instead of name in GRPC status calls [[woodpecker-ci#3143](woodpecker-ci#3143)] - Use UUID instead of step name where possible [[woodpecker-ci#3136](woodpecker-ci#3136)] - Use step type to detect services in Kubernetes backend [[woodpecker-ci#3141](woodpecker-ci#3141)] - Fix config base64 parsing to utf-8 [[woodpecker-ci#3110](woodpecker-ci#3110)] - Pin Gitea version [[woodpecker-ci#3104](woodpecker-ci#3104)] - Fix step `depends_on` as string in schema [[woodpecker-ci#3099](woodpecker-ci#3099)] - Fix slice unmarshaling [[woodpecker-ci#3097](woodpecker-ci#3097)] - Allow PR secrets to be used on close [[woodpecker-ci#3084](woodpecker-ci#3084)] - make event in pipeline schema also a constraint_list [[woodpecker-ci#3082](woodpecker-ci#3082)] - Fix badge's repoUrl with rootpath [[woodpecker-ci#3076](woodpecker-ci#3076)] - Load changed files for closed PR [[woodpecker-ci#3067](woodpecker-ci#3067)] - Fix build output paths [[woodpecker-ci#3065](woodpecker-ci#3065)] - Fix `when` and `depends_on` [[woodpecker-ci#3063](woodpecker-ci#3063)] - Fix DAG cycle detection [[woodpecker-ci#3049](woodpecker-ci#3049)] - Fix duplicated icons [[woodpecker-ci#3045](woodpecker-ci#3045)] ### 📈 Enhancement - Retrieve all user repo perms with a single API call [[woodpecker-ci#3211](woodpecker-ci#3211)] - Secured kubernetes backend configuration [[woodpecker-ci#3204](woodpecker-ci#3204)] - Use `assert` for tests [[woodpecker-ci#3201](woodpecker-ci#3201)] - Replace `goimports` with `gci` [[woodpecker-ci#3202](woodpecker-ci#3202)] - Remove multipart logger [[woodpecker-ci#3200](woodpecker-ci#3200)] - Added protocol in port configuration [[woodpecker-ci#2993](woodpecker-ci#2993)] - Kubernetes AppArmor and seccomp [[woodpecker-ci#3123](woodpecker-ci#3123)] - `cli exec`: let override existing environment values but print a warning [[woodpecker-ci#3140](woodpecker-ci#3140)] - Enable golangci linter forcetypeassert [[woodpecker-ci#3168](woodpecker-ci#3168)] - Enable golangci linter contextcheck [[woodpecker-ci#3170](woodpecker-ci#3170)] - Remove panic recovering [[woodpecker-ci#3162](woodpecker-ci#3162)] - More docker backend test remove more undocumented [[woodpecker-ci#3156](woodpecker-ci#3156)] - Lowercase all log strings [[woodpecker-ci#3173](woodpecker-ci#3173)] - Cleanups + prefer .yaml [[woodpecker-ci#3069](woodpecker-ci#3069)] - Use UUID as podName and cleanup arguments for Kubernetes backend [[woodpecker-ci#3135](woodpecker-ci#3135)] - Enable golangci linter stylecheck [[woodpecker-ci#3167](woodpecker-ci#3167)] - Clean up logging [[woodpecker-ci#3161](woodpecker-ci#3161)] - Enable `gocritic` and don't ignore globally [[woodpecker-ci#3159](woodpecker-ci#3159)] - Remove steps for publishing release branches [[woodpecker-ci#3125](woodpecker-ci#3125)] - Enable `nolintlint` [[woodpecker-ci#3158](woodpecker-ci#3158)] - Enable some linters [[woodpecker-ci#3129](woodpecker-ci#3129)] - Use name in backend types instead of alias [[woodpecker-ci#3142](woodpecker-ci#3142)] - Make service icon rotate [[woodpecker-ci#3149](woodpecker-ci#3149)] - Add step name as label to docker containers [[woodpecker-ci#3137](woodpecker-ci#3137)] - Use js-base64 on pipeline log page [[woodpecker-ci#3146](woodpecker-ci#3146)] - Flexible image pull secret reference [[woodpecker-ci#3016](woodpecker-ci#3016)] - Always show pipeline step list [[woodpecker-ci#3114](woodpecker-ci#3114)] - Add loading spinner and no pull request text [[woodpecker-ci#3113](woodpecker-ci#3113)] - Fix timeout settings contrast [[woodpecker-ci#3112](woodpecker-ci#3112)] - Unfold workflow when opening via URL [[woodpecker-ci#3106](woodpecker-ci#3106)] - Remove env argument of addons [[woodpecker-ci#3100](woodpecker-ci#3100)] - Move `cmd/common` to `shared` [[woodpecker-ci#3092](woodpecker-ci#3092)] - use semver for version comparsion [[woodpecker-ci#3042](woodpecker-ci#3042)] - Extend create plugin docs [[woodpecker-ci#3062](woodpecker-ci#3062)] - Remove old files [[woodpecker-ci#3077](woodpecker-ci#3077)] - Indicate if step is service [[woodpecker-ci#3078](woodpecker-ci#3078)] - Add imports checks to linter [[woodpecker-ci#3056](woodpecker-ci#3056)] - Remove workflow version again [[woodpecker-ci#3052](woodpecker-ci#3052)] - Add option to disable version check in admin web UI [[woodpecker-ci#3040](woodpecker-ci#3040)] ### Misc - chore(deps): update docker.io/woodpeckerci/plugin-docker-buildx docker tag to v3 [[woodpecker-ci#3229](woodpecker-ci#3229)] - Docs: Fix expression syntax docs url [[woodpecker-ci#3208](woodpecker-ci#3208)] - Add schema test for depends_on [[woodpecker-ci#3205](woodpecker-ci#3205)] - chore(deps): lock file maintenance [[woodpecker-ci#3190](woodpecker-ci#3190)] - Do not run prettier with pre-commit [[woodpecker-ci#3196](woodpecker-ci#3196)] - fix(deps): update module github.com/google/go-github/v57 to v58 [[woodpecker-ci#3187](woodpecker-ci#3187)] - chore(deps): update docker.io/golang docker tag to v1.21.6 [[woodpecker-ci#3189](woodpecker-ci#3189)] - chore(deps): update docker.io/woodpeckerci/plugin-docker-buildx [[woodpecker-ci#3186](woodpecker-ci#3186)] - fix(deps): update golang (packages) [[woodpecker-ci#3185](woodpecker-ci#3185)] - declare different when statements once and reuse them [[woodpecker-ci#3176](woodpecker-ci#3176)] - Add `make clean-all` [[woodpecker-ci#3152](woodpecker-ci#3152)] - Fix `version.json` updates [[woodpecker-ci#3057](woodpecker-ci#3057)] - [pre-commit.ci] pre-commit autoupdate [[woodpecker-ci#3101](woodpecker-ci#3101)] - Update dependency @vitejs/plugin-vue to v5 [[woodpecker-ci#3074](woodpecker-ci#3074)] - Use CI vars for plugin [[woodpecker-ci#3061](woodpecker-ci#3061)] - Use `yamllint` [[woodpecker-ci#3066](woodpecker-ci#3066)] - Use dag in ci config [[woodpecker-ci#3010](woodpecker-ci#3010)]

qwerty287 added 2 commits January 10, 2024 12:11

Enable gocritic and don't ignore globally

0556002

Merge branch 'main' into gocritic

4bf4344

qwerty287 added the refactor delete or replace old code label Jan 10, 2024

qwerty287 added this to the 2.2.0 milestone Jan 10, 2024

qwerty287 requested a review from a team January 10, 2024 11:14

qwerty287 added 2 commits January 10, 2024 14:07

Return error instead of log.Fatal

bb6af67

Merge branch 'main' into gocritic

bc590f2

xoxys approved these changes Jan 10, 2024

View reviewed changes

6543 approved these changes Jan 10, 2024

View reviewed changes

6543 merged commit 12c40eb into woodpecker-ci:main Jan 10, 2024
7 of 8 checks passed

woodpecker-bot mentioned this pull request Jan 10, 2024

🎉 Release 2.2.0 #3046

Merged

1 task

qwerty287 deleted the gocritic branch January 10, 2024 14:43

fernandrone pushed a commit to quintoandar/woodpecker that referenced this pull request Feb 1, 2024

Enable gocritic and don't ignore globally (woodpecker-ci#3159)

5b72b75

Use `nolint` directives instead. From woodpecker-ci#2960

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable `gocritic` and don't ignore globally #3159

Enable `gocritic` and don't ignore globally #3159

qwerty287 commented Jan 10, 2024

xoxys commented Jan 10, 2024

woodpecker-bot commented Jan 10, 2024 •

edited

Loading

qwerty287 commented Jan 10, 2024

codecov bot commented Jan 10, 2024 •

edited

Loading

qwerty287 commented Jan 10, 2024

xoxys commented Jan 10, 2024

xoxys commented Jan 10, 2024 •

edited

Loading

qwerty287 commented Jan 10, 2024

xoxys left a comment

Enable gocritic and don't ignore globally #3159

Enable gocritic and don't ignore globally #3159

Conversation

qwerty287 commented Jan 10, 2024

xoxys commented Jan 10, 2024

woodpecker-bot commented Jan 10, 2024 • edited Loading

qwerty287 commented Jan 10, 2024

codecov bot commented Jan 10, 2024 • edited Loading

Codecov Report

qwerty287 commented Jan 10, 2024

xoxys commented Jan 10, 2024

xoxys commented Jan 10, 2024 • edited Loading

qwerty287 commented Jan 10, 2024

xoxys left a comment

Choose a reason for hiding this comment

Enable `gocritic` and don't ignore globally #3159

Enable `gocritic` and don't ignore globally #3159

woodpecker-bot commented Jan 10, 2024 •

edited

Loading

codecov bot commented Jan 10, 2024 •

edited

Loading

xoxys commented Jan 10, 2024 •

edited

Loading