Feat/clarify or expand GitHub ci default ecosystem support#16
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extends Limier's GitHub CI integration to support generic dependency review presets for pip and cargo ecosystems. It adds scenario and fixture generation logic, updates docker shell invocation, and documents the expanded preset coverage alongside existing npm support. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/preset/assets/scenarios/pip-ci.yml (1)
31-41: CatchException(orImportError) instead ofBaseException.
BaseExceptionalso capturesSystemExitandKeyboardInterrupt, which an import probe shouldn't silently swallow.ImportError/ModuleNotFoundErroris the targeted failure mode here;Exceptionis a safer broad fallback if you want to also catch e.g. side‑effect errors during import.♻️ Proposed tightening
for candidate in candidates: try: importlib.import_module(candidate) - except BaseException as exc: + except Exception as exc: print(f"limier fixture: import {candidate!r} failed: {exc}") continue print(f"limier fixture: imported {candidate!r}") break else: print("limier fixture: package installed; no generic import succeeded")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/preset/assets/scenarios/pip-ci.yml` around lines 31 - 41, The import probe currently catches BaseException (in the loop using importlib.import_module(candidate)), which incorrectly swallows SystemExit/KeyboardInterrupt; change the except clause to catch narrower exceptions such as Exception (or specifically ImportError/ModuleNotFoundError) — e.g. replace "except BaseException as exc" with "except Exception as exc" or "except (ImportError, ModuleNotFoundError) as exc" so import failures and runtime import errors are handled but system interrupts are not suppressed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/preset/preset.go`:
- Line 19: The pip package regex pipPackagePattern must be tightened to require
the first and last characters be alphanumeric (e.g. change to something like
^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$) so names cannot start with '-' '.'
or '_'; likewise update the Cargo name validation to require the first character
be a letter (adjust the per-character allowlist/validation used for Cargo names
to enforce leading [A-Za-z] and then allow [A-Za-z0-9_-]* for the rest). Update
the tests in TestResolveFixtureRejectsUnsafeGeneratedPackageNames to add
negative cases such as "-rfoo" for pip and "-serde" for cargo to ensure the new
validators reject these inputs.
---
Nitpick comments:
In `@internal/preset/assets/scenarios/pip-ci.yml`:
- Around line 31-41: The import probe currently catches BaseException (in the
loop using importlib.import_module(candidate)), which incorrectly swallows
SystemExit/KeyboardInterrupt; change the except clause to catch narrower
exceptions such as Exception (or specifically ImportError/ModuleNotFoundError) —
e.g. replace "except BaseException as exc" with "except Exception as exc" or
"except (ImportError, ModuleNotFoundError) as exc" so import failures and
runtime import errors are handled but system interrupts are not suppressed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8f05a6a-cca3-42fe-97cb-20212630ee3a
📒 Files selected for processing (15)
cmd/ci.gocmd/ci_test.gocmd/run.godocs/guide/ci-and-deploy.mddocs/guide/review-your-own-project.mddocs/reference/cli.mdinternal/adapters/cargo/cargo.gointernal/adapters/cargo/cargo_test.gointernal/env/docker/manager.gointernal/env/docker/manager_linux_test.gointernal/env/docker/manager_test.gointernal/preset/assets/scenarios/cargo-ci.ymlinternal/preset/assets/scenarios/pip-ci.ymlinternal/preset/preset.gointernal/preset/preset_test.go
💤 Files with no reviewable changes (1)
- cmd/ci.go
oneslash
deleted the
feat/clarify-or-expand-github-ci-default-ecosystem-support
branch
1 participant
Summary by CodeRabbit
New Features
pipandcargoecosystems (in addition tonpm).npm,pip, andcargowithout requiring custom fixtures or scenarios.Bug Fixes
npmecosystems from using default presets.Documentation
Chores