feat: better printing of dependency diffs - #25
Conversation
Make dependency changes visible without requiring verbose logging so package updates clearly show before-and-after versions. Add reusable semver diff formatting to keep dependency output consistent across update paths.
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughThe changes introduce a dependency update reporting infrastructure by adding an output writer field to the Context struct, creating a DependencyUpdate type to track version changes, updating dependency processing to collect and report these updates, and adding a Diff method to Semver for formatting version comparisons. Changes
Sequence DiagramsequenceDiagram
participant Caller
participant ProcessNPM as processNPMPackage()
participant UpdateDeps as updateDependencies()
participant Print as printDependencyUpdates()
participant Output as Context.Output
Caller->>ProcessNPM: process package (dry-run or actual)
ProcessNPM->>UpdateDeps: update dependencies list
UpdateDeps->>UpdateDeps: fetch versions & compare
UpdateDeps-->>ProcessNPM: return []DependencyUpdate, error
ProcessNPM->>UpdateDeps: update devDependencies list
UpdateDeps-->>ProcessNPM: return []DependencyUpdate, error
ProcessNPM->>Print: printDependencyUpdates(deps updates)
Print->>Print: outputWriter() → get Context.Output
Print->>Output: write formatted update report
ProcessNPM->>Print: printDependencyUpdates(devDeps updates)
Print->>Output: write formatted update report
ProcessNPM-->>Caller: complete with updates reported
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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)
📝 Coding Plan
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 |
Route visible dependency diff messages through the existing log package so normal output follows the project's logging infrastructure. Keep tests deterministic by injecting a test logger instead of writing directly to stdout.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
npm.go (2)
126-132: The nil check is redundant given the default initialization.
Ctx.Outputis initialized toos.Stdoutinctx.go, soCtx.Outputwill never benilin practice. The fallback is defensive but unnecessary.♻️ Suggested simplification
func outputWriter() io.Writer { - if Ctx.Output != nil { - return Ctx.Output - } - - return os.Stdout + return Ctx.Output }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm.go` around lines 126 - 132, The nil check in outputWriter() is unnecessary because Ctx.Output is default-initialized to os.Stdout in ctx.go; simplify the function by removing the conditional and directly return Ctx.Output (reference symbols: function outputWriter and variable Ctx.Output, and initialization in ctx.go) so the function is a single return of Ctx.Output.
166-202: Returningnilon error discards partial updates.When
getNPMPackageLatestVersionfails mid-iteration (line 178), any successfully processed updates are discarded by returningnil. This matches the test expectations but may lose useful information about which dependencies were successfully evaluated before the failure.If partial visibility is desired, consider returning
updatesinstead ofnilon error. However, the current behavior is consistent and acceptable if the intent is all-or-nothing semantics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm.go` around lines 166 - 202, The function updateDependencies currently returns nil on error from getNPMPackageLatestVersion, discarding any partial updates; change the error return to return the collected updates along with the error (i.e. return updates, fmt.Errorf(...)) so callers can see which DependencyUpdate entries were produced before the failure; update any tests or callers that expect all-or-nothing behavior if needed and keep references to updateDependencies, getNPMPackageLatestVersion, and DependencyUpdate when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@npm.go`:
- Around line 126-132: The nil check in outputWriter() is unnecessary because
Ctx.Output is default-initialized to os.Stdout in ctx.go; simplify the function
by removing the conditional and directly return Ctx.Output (reference symbols:
function outputWriter and variable Ctx.Output, and initialization in ctx.go) so
the function is a single return of Ctx.Output.
- Around line 166-202: The function updateDependencies currently returns nil on
error from getNPMPackageLatestVersion, discarding any partial updates; change
the error return to return the collected updates along with the error (i.e.
return updates, fmt.Errorf(...)) so callers can see which DependencyUpdate
entries were produced before the failure; update any tests or callers that
expect all-or-nothing behavior if needed and keep references to
updateDependencies, getNPMPackageLatestVersion, and DependencyUpdate when making
the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33767025-1c8e-4949-ad7b-47a0d649b94f
📒 Files selected for processing (5)
ctx.gonpm.gonpm_test.gosemver.gosemver_test.go
- Change log output from Info to Debug level for dependency version checks and up-to-date reporting. - Remove redundant update action log, streamlining console output during dependency updates. - No changes to update logic; only logging/moderation of output for improved clarity.
1 participant
Summary
Semver.Diffhelper so semver changes can be formatted consistentlyCloses #16
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes