Skip to content

Conversation

matthewdale
Copy link
Collaborator

@matthewdale matthewdale commented Aug 1, 2024

GODRIVER-3293

Summary

  • Upgrade golangci-lint to 1.59.1 and fix lint errors.
  • Change many instances of log.Fatal to log.Panic.
    • One of the new lint checks points out that calling log.Fatal after calling defer prevents the defer from running because log.Fatal exits the application immediately. In those cases, use log.Panic, which has the same logging behavior but panics instead, which still allows deferred calls to execute.

Background & Motivation

Our current golangci-lint (v1.52.2) panics with Go 1.22. Upgrade it to latest (v1.59.1) to resolve the panic and to take advantage of improved lint checks.

This PR depends on #1730 to upgrade Go to 1.22 because golangci-lint v1.59.1 won't build with Go 1.20.

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the review-priority-low Low Priority PR for Review: within 3 business days label Aug 1, 2024
@matthewdale matthewdale marked this pull request as draft August 2, 2024 01:27
@matthewdale matthewdale force-pushed the godriver3293-upgrade-lint branch from dbde20d to b251d74 Compare August 2, 2024 05:14
@matthewdale matthewdale force-pushed the godriver3293-upgrade-lint branch from b251d74 to a47d652 Compare August 9, 2024 19:27
@matthewdale matthewdale marked this pull request as ready for review August 9, 2024 19:27
Copy link
Contributor

API Change Report

No changes found!

gocritic:
enabled-checks:
# Detects suspicious append result assignments. E.g. "b := append(a, 1, 2, 3)"
- appendAssign
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it covered by default in the new version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes it is! I got a warning that the config is redundant:

WARN [linters_context] gocritic: no need to enable check "appendAssign": it's already enabled 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-priority-low Low Priority PR for Review: within 3 business days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants