Update golangci-lint #147
Update golangci-lint #147
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request updates the golangci-lint workflow configuration and adjusts several function signatures to use underscore parameters for unused values. It also refactors conditional branches in subscription handling from if/else constructs to a switch-case for clarity and updates CI workflow inputs for Golang and golangci-lint versions.
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/service/save_subscription_test.go | Updated monkey patch signatures for clarity. |
| server/service/notification_test.go | Updated unused parameter handling in monkey patches. |
| server/service/delete_subscription_test.go | Consistent change to ignore unused parameters. |
| server/service/confluence_service.go | Adjusted CheckConfluenceURL signature to ignore HTTPS. |
| server/serializer/confluence_server.go | Modified GetNotificationPost signature. |
| server/save_subscription.go | Replaced if/else with switch-case for subscriptions. |
| server/plugin.go | Updated ExecuteCommand and ServeHTTP signatures. |
| server/flow.go | Modified callback parameter naming for clarity. |
| server/edit_subscription.go | Similar switch-case and underscore changes applied. |
| server/controller.go | Refactored looping construct in verifyHTTPSecret. |
| server/command.go | Updated command functions to use underscore for unused parameters. |
| build/pluginctl/logs.go | Adjusted error message formatting and linter suppression. |
| .golangci.yml | Updated configuration structure for new golangci-lint version. |
| .github/workflows/ci.yml | Added new workflow inputs for golang and golangci-lint versions. |
Files not reviewed (1)
- Makefile: Language not supported
Comments suppressed due to low confidence (1)
server/service/confluence_service.go:59
- [nitpick] If the HTTPS requirement parameter is no longer used, consider updating the documentation or removing it entirely from the function signature if possible to simplify the API.
func CheckConfluenceURL(mattermostSiteURL, confluenceURL string, _ bool) (string, error) {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
.github/workflows/ci.yml
Outdated
| @@ -9,3 +9,6 @@ jobs: | |||
| plugin-ci: | |||
| uses: mattermost/actions-workflows/.github/workflows/plugin-ci.yml@f53a9d32cc670c0319d95332e07c68c00b332928 | |||
| secrets: inherit | |||
| with: | |||
| golang-version: 1.24 | |||
There was a problem hiding this comment.
You are on go1.22.8
mattermost-plugin-confluence/go.mod
Line 5 in 0d2815f
But your CI was on go1.21: https://github.com/mattermost/actions-workflows/blob/2174576d3c65eb4db691bf09fd72246b59f331c8/.github/workflows/plugin-ci.yml#L26
Inside the PR #140, related to the update of github.com/mattermost/mattermost/server/public, you will be on go1.23: https://github.com/mattermost/mattermost/blob/34fb9adbc25157de8cb2a77d95aaa9fd9b97830d/server/go.mod#L5
| @@ -51,7 +51,7 @@ apply: | |||
| ## Install go tools | |||
| install-go-tools: | |||
| @echo Installing go tools | |||
| $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1 | |||
| $(GO) install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.1 | |||
There was a problem hiding this comment.
The other MM projects are on v1.64. This will be the only project using the v2 version. Is this necessary?
There was a problem hiding this comment.
The maintenance of v1 has stopped when the first v2 version was released.
The other MM projects should be upgraded.
FYI, the make target was using 1.51, but the CI was using v1.55 https://github.com/mattermost/actions-workflows/blob/2174576d3c65eb4db691bf09fd72246b59f331c8/.github/workflows/plugin-ci.yml#L19
There was a problem hiding this comment.
Some mentioned recently that upgrading to v2 is on their todo list, so +1 on upgrading.
There was a problem hiding this comment.
Pull Request Overview
This PR updates golangci-lint to v2.1.1 and refactors several functions and tests to ignore unused parameters by replacing unused variable names with underscores.
- Refactored monkey patch closures in tests and production code to use underscores for unused parameters
- Updated error handling messages and workflow parameters for CI
- Adjusted subscription handler functions to use switch-case on subscription type
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/service/save_subscription_tes 8000 t.go | Replaced unused closure parameters with underscores in monkey patches |
| server/service/notification_test.go | Updated closure parameters with underscores for unused arguments |
| server/service/delete_subscription_test.go | Updated closure parameters with underscores |
| server/service/confluence_service.go | Refactored CheckConfluenceURL unused parameter to underscore |
| server/serializer/confluence_server.go | Updated GetNotificationPost parameter to underscore |
| server/save_subscription.go | Switched to switch-case for handling subscription type |
| server/plugin.go | Updated ExecuteCommand and ServeHTTP parameter names to underscore where unused |
| server/flow.go | Revised flow step callbacks to ignore unused flow parameter using underscore |
| server/edit_subscription.go | Switched to switch-case for subscription type handling, similar to save_subscription.go |
| server/controller.go | Minor comment improvements in verifyHTTPSecret loop |
| server/command.go | Updated several command execution functions to use underscore for unused variadic arguments |
| build/pluginctl/logs.go | Adjusted linter settings and error message formatting |
| .golangci.yml | Updated configuration to use version 2 with new formatter settings and exclusions |
| .github/workflows/ci.yml | Updated workflow parameters for golang and golangci-lint versions |
Files not reviewed (1)
- Makefile: Language not supported
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR updates the golangci-lint configuration and makes several minor refactorings across the codebase. The changes primarily include:
- Replacing unused function parameters with underscore (_) in tests and service methods.
- Refactoring subscription type handling from if/else constructs to switch-case statements.
- Updating CI workflow and golangci-lint configuration to use explicit versions and new configuration options.
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/save_subscription_test.go, server/service/notification_test.go, server/service/delete_subscription_test.go |
Updated monkey patch functions to use underscore for unused parameters. |
| server/service/confluence_service.go, server/serializer/confluence_server.go |
Revised function signatures to ignore unused parameters with underscore. |
| server/save_subscription.go, server/edit_subscription.go |
Converted if/else blocks to switch-case for subscription handling. |
| server/plugin.go, server/flow.go, server/command.go |
Updated function parameter names to underscore for unused context parameters. |
| server/controller.go | Improved the constant-time comparison loop with an updated comment explaining its behavior. |
| build/pluginctl/logs.go | Adjusted nolint directives with updated static analysis settings. |
| .golangci.yml | Modernized configuration by introducing formatters and updated linters settings for golangci-lint version 2. |
| .github/workflows/ci.yml | Added explicit parameters for golang and golangci-lint versions. |
Files not reviewed (1)
- Makefile: Language not supported
|
/update-branch |
|
Error trying to update the PR. |
|
Thanks @ldez 👍 |
Summary
Update golangci-lint.
Uses the workflow parameters defined in https://github.com/mattermost/actions-workflows/blob/main/.github/workflows/plugin-ci.yml
The modifications work locally with the generated files, but I can't guarantee that this will work correctly on your CI because a file (
server/manifest.go) may not be generated by your CI workflow (I don't know).Ticket Link
golangci/golangci-lint#5760