Skip to content

t4946: eliminate redundant detect_package_manager call in check_requirements#4949

Merged
alex-solovyev merged 1 commit intomainfrom
bugfix/quality-debt-core-sh-4946
Mar 15, 2026
Merged

t4946: eliminate redundant detect_package_manager call in check_requirements#4949
alex-solovyev merged 1 commit intomainfrom
bugfix/quality-debt-core-sh-4946

Conversation

@alex-solovyev
Copy link
Collaborator

@alex-solovyev alex-solovyev commented Mar 15, 2026

Summary

  • Call detect_package_manager once at the top of check_requirements() instead of twice (once inside the sqlite3 check, once in the install block)
  • Unifies the variable name from pkg_mgr / pkg_manager to a single pkg_manager used throughout

Findings addressed

Severity Reviewer Finding Status
HIGH CodeRabbit Homebrew formula is sqlite, not sqlite3 Already fixed in PR #4935 commit 13f05ab
MEDIUM Gemini Redundant detect_package_manager call Fixed in this PR

The HIGH finding was already addressed during PR #4935 — the current code correctly maps apt to sqlite3 and everything else (including brew) to sqlite. This PR addresses the remaining MEDIUM finding.

Changes

  • setup-modules/core.sh: Hoist detect_package_manager call to the top of the dependency-check block, remove the two redundant calls downstream

Closes #4946

Summary by CodeRabbit

  • Refactor
    • Improved package manager detection logic in the setup process for better efficiency and code clarity.

…requirements

Call detect_package_manager once at the top of the dependency-check block
and reuse the result for both sqlite package-name resolution and the
install prompt. Previously it was called twice: once inside the sqlite3
check and again when installing missing deps.

Closes #4946
@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the check_requirements function in setup-modules/core.sh to improve efficiency and code clarity. By centralizing the package manager detection and standardizing the variable name, the changes streamline the dependency checking process and address a previously identified code redundancy.

Highlights

  • Dependency Detection Optimization: The detect_package_manager function is now called only once at the beginning of check_requirements(), eliminating two redundant calls within the function.
  • Variable Name Unification: The variable names pkg_mgr and pkg_manager have been unified to a single pkg_manager for consistency throughout the check_requirements() function.
  • Addressed Finding: This pull request resolves a MEDIUM severity finding from Gemini regarding redundant detect_package_manager calls.
Changelog
  • setup-modules/core.sh
    • Hoisted the detect_package_manager call to the top of the check_requirements function.
    • Removed two redundant calls to detect_package_manager from within the sqlite3 check and the general dependency installation block.
    • Standardized the package manager variable name from pkg_mgr to pkg_manager.
Activity
  • Addressed a MEDIUM severity finding from Gemini regarding redundant detect_package_manager calls.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: edfc124f-d249-46c1-82ff-13074fab4a1a

📥 Commits

Reviewing files that changed from the base of the PR and between 2ea4f89 and 708fffe.

📒 Files selected for processing (1)
  • setup-modules/core.sh

Walkthrough

Refactors check_requirements to centralize package manager detection by calling detect_package_manager once upfront and storing the result in a variable for reuse. Additionally corrects the Homebrew formula mapping for sqlite from sqlite3 to sqlite, ensuring accurate package installation across different package managers.

Changes

Cohort / File(s) Summary
Package Manager Detection Refactoring
setup-modules/core.sh
Consolidates redundant detect_package_manager calls into a single upfront invocation within check_requirements. Replaces locally scoped package manager detection with a reusable pkg_manager variable. Fixes Homebrew sqlite formula mapping from incorrect sqlite3 to correct sqlite in case statement; maintains sqlite3 for apt-based systems and sqlite as fallback for Fedora/Arch/Alpine.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🎯 One detection call instead of many,
Package manager centralized, no need for any—
Sqlite brews as sqlite, not sqlite3,
Redundancy vanished, efficiency's the key! 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and clearly summarizes the main change: eliminating a redundant detect_package_manager call in the check_requirements function, which is exactly what the changeset implements.
Linked Issues check ✅ Passed The PR addresses both objectives from linked issue #4946: it removes the redundant detect_package_manager call by hoisting it to the top of check_requirements and unifies variable naming inconsistencies (pkg_mgr/pkg_manager).
Out of Scope Changes check ✅ Passed All changes are scoped to the stated objectives. The PR focuses solely on the redundant function call elimination and variable naming consistency; the HIGH finding regarding Homebrew's sqlite package name was already addressed in commit 13f05ab of PR #4935.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/quality-debt-core-sh-4946
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@github-actions
Copy link
Contributor

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 362 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Sun Mar 15 18:24:30 UTC 2026: Code review monitoring started
Sun Mar 15 18:24:31 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 362

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 362
  • VULNERABILITIES: 0

Generated on: Sun Mar 15 18:24:33 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively refactors the check_requirements function in setup-modules/core.sh to eliminate redundant calls to detect_package_manager. By calling the function once at the top and reusing its result, the code is now more efficient and readable. The change is well-implemented.

@sonarqubecloud
Copy link

@alex-solovyev alex-solovyev merged commit cd6ca4c into main Mar 15, 2026
28 checks passed
@alex-solovyev alex-solovyev deleted the bugfix/quality-debt-core-sh-4946 branch March 15, 2026 18:41
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.

quality-debt: setup-modules/core.sh — PR #4935 review feedback (high)

1 participant