fix: (dpf/bf4) - Changes to sync BF4 flavor as used by DPF team#3772
Conversation
Summary by CodeRabbit
WalkthroughBF4 flavor generation now adds ChangesBF4 configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/dpf/src/flavor.rs (1)
179-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the BF4 flavor contract.
The selector and helper jointly change the generated NVconfig. Add a focused test asserting the device and exact BF4 parameters, including
PF_SF_BAR_SIZE=14,PF_TOTAL_SF=30, and bothLINK_TYPE_* = ETH, so a future refactor cannot silently fall back to the shared defaults.As per path instructions, prioritize missing behavior coverage for this change. As per coding guidelines, use table-driven cases when covering multiple flavor outputs.
Also applies to: 358-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/dpf/src/flavor.rs` at line 179, In the flavor tests covering the selector and get_bf4_default_nvconfig, add table-driven regression coverage for the BF4 output. Assert the expected device and exact NVconfig values, including PF_SF_BAR_SIZE=14, PF_TOTAL_SF=30, and both LINK_TYPE_* values set to ETH, while preserving coverage for the other flavor outputs.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/dpf/src/flavor.rs`:
- Line 179: In the flavor tests covering the selector and
get_bf4_default_nvconfig, add table-driven regression coverage for the BF4
output. Assert the expected device and exact NVconfig values, including
PF_SF_BAR_SIZE=14, PF_TOTAL_SF=30, and both LINK_TYPE_* values set to ETH, while
preserving coverage for the other flavor outputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: db16eaf2-874b-4c4c-99e4-e16dd9aee194
📒 Files selected for processing (1)
crates/dpf/src/flavor.rs
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3772.docs.buildwithfern.com/infra-controller |
Changes to sync BF4 flavor as used by DPF team
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes