Skip to content

[chore] Updating golangci-lint version and correcting errors and checks - #1797

Merged
rm3l merged 12 commits into
redhat-developer:mainfrom
OpinionatedHeron:golangUpdate
Nov 25, 2025
Merged

[chore] Updating golangci-lint version and correcting errors and checks#1797
rm3l merged 12 commits into
redhat-developer:mainfrom
OpinionatedHeron:golangUpdate

Conversation

@OpinionatedHeron

Copy link
Copy Markdown
Member

Description

Updated the golangci-lint to v2.5.0. Ran 'make lint' and fixed or ignored any errors and checks that were flagged by the linter.

Which issue(s) does this PR fix or relate to

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

Ran 'make lint' until 0 errors returned.
Also ran 'make tests' to ensure tests still worked as expected.

Signed-off-by: Leanne Ahern <lahern@redhat.com>
Signed-off-by: Leanne Ahern <lahern@redhat.com>

@rm3l rm3l left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@OpinionatedHeron Please update your PR branch, so that the CI checks can run correctly. Thanks.

@Fortune-Ndlovu Fortune-Ndlovu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@rm3l

rm3l commented Oct 28, 2025

Copy link
Copy Markdown
Member

/review

@qodo-code-review

Copy link
Copy Markdown
Contributor

You are above your monthly Qodo Merge usage quota. For more information, please visit here.

1 similar comment
@rhdh-qodo-merge

Copy link
Copy Markdown

You are above your monthly Qodo Merge usage quota. For more information, please visit here.

Comment thread integration_tests/matchers.go Outdated
Comment thread internal/controller/backstage_controller.go Outdated
Comment thread pkg/model/deployment.go Outdated
Comment thread pkg/model/deployment.go Outdated
Comment thread tests/e2e/e2e_suite_test.go Outdated
Comment thread tests/e2e/e2e_test.go Outdated
Comment thread tests/e2e/e2e_upgrade_test.go Outdated
Comment thread tests/helper/helper_backstage.go Outdated
Comment thread integration_tests/matchers.go Outdated
@zdrapela

Copy link
Copy Markdown
Member

/review

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Oct 31, 2025

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit fa25b41)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHDHBUGS-599 - Partially compliant

Compliant requirements:

  • Fix linter-reported errors or provide code comments justifying any ignored findings.
  • Ensure make lint passes.

Non-compliant requirements:

  • Bump golangci-lint to the latest available version (v2.1.6 at ticket time).

Requires further human verification:

  • Verify that make lint passes in CI with updated toolchain on clean environment.
  • Confirm that upgrading to golangci-lint v2.5.0 (newer than v2.1.6) is acceptable per ticket scope, or align to exactly the "latest available" per ticket at merge time.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Ticket Mismatch

The ticket specifies "latest available version (v2.1.6 at the time of writing)". PR bumps to v2.5.0. Confirm product decision that using a newer v2 is acceptable or adjust to the ticket-stated version.

GINKGO ?= $(LOCALBIN)/ginkgo-$(GINKGO_VERSION)

## Tool Versions
KUSTOMIZE_VERSION ?= v5.4.2
CONTROLLER_TOOLS_VERSION ?= v0.14.0
ENVTEST_VERSION ?= release-0.17
GOLANGCI_LINT_VERSION ?= v2.5.0
GOIMPORTS_VERSION ?= v0.16.1
GOSEC_VERSION ?= v2.22.8
GINKGO_VERSION ?= v2.22.2
Behavior Change

Switched from r.Client.Get to embedded r.Get. Ensure reconciler embeds client.Client; otherwise this could change behavior or break dependency injection in tests.

func (r *BackstageReconciler) getOCPIngressDomain() (string, error) {
	var u unstructured.Unstructured
	u.SetGroupVersionKind(schema.GroupVersionKind{
		Group:   "config.openshift.io",
		Kind:    "Ingress",
		Version: "v1",
	})

	err := r.Get(context.Background(), client.ObjectKey{
		Name:      "cluster",
		Namespace: "",
	}, &u)
	if err != nil {
Nil Map Access

Accessing annotations via deploy.Spec.Template.GetAnnotations() is safer but confirm no unintended behavior if annotations are nil; ensure equality logic still works as expected for empty maps.

		lg.V(1).Info("request by label, deployment not found", "name", model.DeploymentName(backstage.Name))
	} else {
		lg.Error(err, "request by label failed, get Deployment ", "error ", err)
	}
	return []reconcile.Request{}
}

newHash := ec.WatchingHash
oldHash := deploy.Spec.Template.GetAnnotations()[model.ExtConfigHashAnnotation]
if newHash == oldHash {
	lg.V(1).Info("request by label, hash are equal", "hash", newHash)
	return []reconcile.Request{}
}
📄 References
  1. No matching references available

@qodo-code-review

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit fa25b41

@Fortune-Ndlovu Fortune-Ndlovu removed their assignment Nov 3, 2025
Comment thread Makefile Outdated
Comment thread .golangci.yml Outdated
Comment thread .golangci.yml Outdated
Signed-off-by: Leanne Ahern <lahern@redhat.com>
Comment thread .golangci.yml Outdated
Signed-off-by: Leanne Ahern <lahern@redhat.com>
Comment thread .golangci.yml Outdated
Signed-off-by: Leanne Ahern <lahern@redhat.com>

@rm3l rm3l left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your work on this !!

/lgtm

Comment thread .golangci.yml
Comment on lines +6 to +7
enable:
- staticcheck

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: staticcheck should already be part of the linters enabled by default.

@openshift-ci

openshift-ci Bot commented Nov 25, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Fortune-Ndlovu, rm3l

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rm3l
rm3l merged commit b6dd6d6 into redhat-developer:main Nov 25, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants