Skip to content

Fix setting dep label selector#836

Open
eemcmullan wants to merge 1 commit into
konveyor:mainfrom
eemcmullan:dep-label
Open

Fix setting dep label selector#836
eemcmullan wants to merge 1 commit into
konveyor:mainfrom
eemcmullan:dep-label

Conversation

@eemcmullan

@eemcmullan eemcmullan commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #803

Summary by CodeRabbit

  • Refactor

    • Reorganized dependency label selector derivation logic to use a centralized helper function for improved code maintainability.
  • Tests

    • Added unit tests to validate dependency label selector behavior for known libraries analysis.

@eemcmullan
eemcmullan requested a review from jmle May 28, 2026 18:34
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@eemcmullan, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes and 53 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8e622513-6b92-4190-bc9e-8657d4064b00

📥 Commits

Reviewing files that changed from the base of the PR and between 9083116 and 742b1aa.

📒 Files selected for processing (3)
  • cmd/analyze/run.go
  • pkg/labels/util.go
  • pkg/labels/util_test.go
📝 Walkthrough

Walkthrough

This PR consolidates dependency label selector construction into a reusable helper function, extracting logic previously inlined in the analyze command. A new DepLabelSelectorForAnalysis function in pkg/labels/util.go generates selectors conditionally, with test coverage added and integration simplified in cmd/analyze/run.go through import cleanup and unconditional option wiring.

Changes

Label Selector Helper Consolidation

Layer / File(s) Summary
Core helper function for dependency label selector
pkg/labels/util.go
Introduces DepLabelSelectorForAnalysis(analyzeKnownLibraries bool) string that returns a dependency label selector string based on the known libraries flag, delegating to provider.DepSourceLabel for the underlying label expressions.
Test for label selector helper
pkg/labels/util_test.go
Adds TestDepLabelSelectorForAnalysis with parallel subtests validating selector strings for both analyzeKnownLibraries=false (excludes open-source) and analyzeKnownLibraries=true (tautology selector) cases.
Integration and import cleanup in run.go
cmd/analyze/run.go
Removes provider import, adds labels import, replaces inline selector construction with labels.DepLabelSelectorForAnalysis(a.analyzeKnownLibraries), and changes analyzer wiring to unconditionally append WithDepLabelSelector(depLabelSelector).

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A label helper hops into view,
Selector logic split clean and true,
No more conditionals cluttering the way,
Tests guard the path, all refactored today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: refactoring how the dependency label selector is set by moving logic to a helper function.
Linked Issues check ✅ Passed The PR addresses issue #803 by removing the condition that caused the erroneous log message and always wiring the dep label selector into analyzer options.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the dep label selector issue: refactoring selector logic into a helper function and removing conditional wiring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Actionable comments posted: 0

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.51%. Comparing base (7e1c79d) to head (742b1aa).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
cmd/analyze/run.go 0.00% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (71.42%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #836      +/-   ##
==========================================
+ Coverage   56.43%   56.51%   +0.07%     
==========================================
  Files          61       61              
  Lines        4896     4898       +2     
==========================================
+ Hits         2763     2768       +5     
+ Misses       1821     1818       -3     
  Partials      312      312              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Emily McMullan <emcmulla@redhat.com>
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.

error msg=\"could not construct dep label selector from condition context, search scope will not be limited"

1 participant