chore: enable errcheck, nolintlint, and nilnil linters [SDCICD-1266] #3111
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christophermancini The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Enable three new golangci-lint checks to improve code quality: **errcheck** - Detects unchecked errors that could lead to silent failures. Ensures all error returns are explicitly handled or intentionally ignored. Changes: - Wrapped async audit logging call in closure to explicitly discard error (audit logging is fire-and-forget to avoid blocking sanitization) **nolintlint** - Validates proper formatting and usage of //nolint directives. Prevents malformed suppressions and removes unused directives. Changes: - Fixed //nolint formatting in cluster.go and cmd.go (removed spaces) - Excluded pkg/common/concurrentviper/generated.go from linting in config (file is generated but generator is currently broken) **nilnil** - Prevents functions from returning (nil, nil) which is ambiguous. Callers cannot distinguish between success-with-no-data and error states. Changes: - Added ErrS3Disabled sentinel error for disabled S3 upload - Updated NewS3Uploader caller to check for ErrS3Disabled - Changed clusterutil.go to return ErrReserveFull instead of (nil, nil) - Changed terraform destroy to return empty map instead of nil All changes maintain existing behavior while making error handling and API contracts more explicit and type-safe. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
christophermancini
force-pushed
the
cm/errcheck
branch
from
eae0aa2 to
187180c
Compare
| @@ -256,11 +257,11 @@ func (o *E2EOrchestrator) uploadToS3() error { | |||
| component := deriveComponentFromTestImage() | |||
| uploader, err := aws.NewS3Uploader(component) | |||
There was a problem hiding this comment.
| uploader, err := aws.NewS3Uploader(component) | |
| if viper.GetString(config.Tests.LogBucket)!=""{ | |
| uploader, err := aws.NewS3Uploader(component) | |
| } else{ | |
| return nil | |
| } |
| bucket := viper.GetString(config.Tests.LogBucket) | ||
| if bucket == "" { | ||
| // S3 upload disabled - no bucket configured | ||
| return nil, nil | ||
| return nil, ErrS3Disabled | ||
| } |
There was a problem hiding this comment.
this whole thing can be removed, see https://github.com/openshift/osde2e/pull/3111/changes#r2789748662
| if errors.Is(err, ErrReserveFull) { | ||
| log.Printf("Reserve full, exiting without provisioning") | ||
| return nil, nil | ||
| return nil, ErrReserveFull |
There was a problem hiding this comment.
is this returned error being handled? we would not want the execution to error out because of this new return value.
|
@christophermancini: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions 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-sigs/prow repository. |
3 participants
Enable three new golangci-lint checks to improve code quality:
errcheck - Detects unchecked errors that could lead to silent failures. Ensures all error returns are explicitly handled or intentionally ignored.
Changes:
nolintlint - Validates proper formatting and usage of //nolint directives. Prevents malformed suppressions and removes unused directives.
Changes:
nilnil - Prevents functions from returning (nil, nil) which is ambiguous. Callers cannot distinguish between success-with-no-data and error states.
Changes:
All changes maintain existing behavior while making error handling and API contracts more explicit and type-safe.