Require approval from the nat-dep-approvers group for dependency changes#782
Conversation
Signed-off-by: David Gardner <dagardner@nvidia.com>
WalkthroughUpdated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-08-28T23:22:41.742ZApplied to files:
🔇 Additional comments (1)
✨ Finishing Touches🧪 Generate 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/CODEOWNERS (2)
5-5: Confirm scope: root-only vs any uv.lockUnanchored pattern matches in any folder. If you intend to gate only the root lockfile, anchor it.
Apply this if root-only was intended:
-uv.lock @nvidia/nat-dep-approvers +/uv.lock @nvidia/nat-dep-approvers
4-5: Optional: Cover other dep manifests if presentIf the repo includes any of these, consider adding them to the same approver group: requirements*.txt, constraints*.txt, Pipfile, Pipfile.lock, poetry.lock, environment*.yml (Conda), conda-lock*.yaml.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/CODEOWNERS(1 hunks)
🔇 Additional comments (2)
.github/CODEOWNERS (2)
4-5: LGTM: Specific rule for uv.lock correctly overrides the default ownersPlacement after the catch‑all and file‑specific pattern are both correct.
4-5: Verify team and branch protection settings
Teamnat-dep-approverswas not found in the NVIDIA org (HTTP 404). Confirm the correct team slug, ensure it has write access to the repo, and verify that “Require review from Code Owners” is enabled on thedevelopbranch.
Signed-off-by: David Gardner <dagardner@nvidia.com>
|
/merge |
By Submitting this PR I confirm:
Summary by CodeRabbit