Skip to content

fix: remove deadcode#2471

Closed
kristina-solovyova wants to merge 1 commit into03-24-fix_address_linting_alertsfrom
04-20-fix_remove_deadcode
Closed

fix: remove deadcode#2471
kristina-solovyova wants to merge 1 commit into03-24-fix_address_linting_alertsfrom
04-20-fix_remove_deadcode

Conversation

@kristina-solovyova
Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions
Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown
Collaborator Author

kristina-solovyova commented Apr 20, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label main-merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kristina-solovyova kristina-solovyova marked this pull request as ready for review April 20, 2026 09:30
@kristina-solovyova kristina-solovyova requested a review from a team as a code owner April 20, 2026 09:30
@graphite-app graphite-app bot requested review from assafgi and tigrawap April 20, 2026 09:30
@claude
Copy link
Copy Markdown

claude bot commented Apr 20, 2026

Claude finished @kristina-solovyova's task in 2m 37s —— View job


PR Review: fix: remove deadcode

  • Gather context and read parent PR diff
  • Analyze non-trivial changes (reconciler_loop.go, funcs_upgrade.go, funcs_smbw.go)
  • Verify removed code is truly unused
  • Check for any correctness issues in refactored code
  • Post final review

Summary

This is a clean dead-code removal PR (-675 net lines). All removed functions were verified to have zero call sites. The non-trivial refactors are correct. Two minor issues below.


Issues

1. Stale comment referencing deleted HasFeatureFlagsinternal/services/feature_flags_cache.go:38,40

After deleting HasFeatureFlags, the FeatureFlagsCache var's deprecation comment still references it:

// the package-level functions GetFeatureFlags, SetFeatureFlags, HasFeatureFlags.
//
// Deprecated: Use GetFeatureFlags, SetFeatureFlags, HasFeatureFlags instead.

HasFeatureFlags no longer exists. The comment should be updated to only mention GetFeatureFlags and SetFeatureFlags.

Fix this →

2. Duplicate //nolint:errcheck directives — internal/controllers/wekacluster/steps_deletion.go:248,256,272

When updating the nolint comment text, the old directive was kept alongside the new one, producing:

_ = r.RecordEventThrottled(...) //nolint:errcheck // event recording errors are intentionally ignored //nolint:errcheck // error is intentionally ignored

The second //nolint:errcheck is redundant. Should be a single directive with one explanation.

Fix this →


Verified Correct

  • HandleManualPause removal: Was never wired as a step Run function in the base branch — genuinely dead.
  • funcs_upgrade.go variable shadowing fixes: status, err := ...var status ...; status, err = ... and inner err := r.prepareForUpgradeS3(...)err = ... are correct fixes to variable shadowing.
  • CreateSmbwCluster(ctx, *SmbwParams): Interface + impl + call site all consistently updated to pointer.
  • GetDriveTypesRatio removal: Zero call sites — dead code.
  • contains simplification (len(s) > 0s != ""): Semantically equivalent.
  • reconciler_loop.go multi-arg append: Pure cosmetic refactor, no logic change.
  • All other removed functions (GetCluster, GetAllClusters, GetAllContainers, GetOperatorPod, CompareTolerations, FilterContainersForDeletion, Owner.MarshalYAML/UnmarshalYAML, etc.) confirmed to have zero remaining references.

@graphite-app
Copy link
Copy Markdown

graphite-app bot commented Apr 20, 2026

Graphite Automations

"Add anton/matt/sergey/kristina as reviwers on operator PRs" took an action on this PR • (04/20/26)

2 reviewers were added to this PR based on Anton Bykov's automation.

@weka weka deleted a comment from claude bot Apr 20, 2026
@graphite-app
Copy link
Copy Markdown

graphite-app bot commented Apr 20, 2026

Merge activity

  • Apr 20, 9:39 AM UTC: kristina-solovyova added this pull request to the Graphite merge queue.
  • Apr 20, 9:39 AM UTC: CI is running for this pull request on a draft pull request (#2472) due to your merge queue CI optimization settings.
  • Apr 20, 10:15 AM UTC: Merged by the Graphite merge queue via draft PR: #2472.

@graphite-app graphite-app bot closed this Apr 20, 2026
@graphite-app graphite-app bot deleted the 04-20-fix_remove_deadcode branch April 20, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants