enhancement: add semver-support - #23
Conversation
|
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. 📝 WalkthroughWalkthroughThis PR introduces comprehensive semver support to the codebase. A new Changes
Sequence DiagramsequenceDiagram
participant updateDeps as updateDependencies
participant parseVer as parseDependencyVersion
participant classify as classifyDependencyUpdate
participant merge as mergeDependencyVersion
participant store as DependencyJSON.Version
updateDeps->>parseVer: current version string
parseVer-->>updateDeps: DependencyVersion (parsed semver)
updateDeps->>parseVer: latest version string (from npm)
parseVer-->>updateDeps: DependencyVersion (parsed semver)
updateDeps->>classify: currentVersion, latestVersion
classify-->>updateDeps: SemverChange, shouldUpdate bool
alt shouldUpdate
updateDeps->>merge: currentVersion, latestVersion
merge-->>updateDeps: merged DependencyVersion
updateDeps->>store: store merged version
else downgrade or noop
updateDeps->>store: skip (preserve current)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
npm.go (2)
69-77: Consider edge case: standalone*version.The prefix list includes
*, but when the version is exactly*(a valid npm wildcard meaning "any version"), this extracts*as a prefix with an empty core version, which will fail semver parsing and fall back to raw. This is probably fine since*isn't a valid semver, but verify this matches expected behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm.go` around lines 69 - 77, extractDependencyVersionPrefix currently treats a standalone "*" as a prefix and returns ("*", ""), which leads to an empty core version and breaks downstream semver parsing; change extractDependencyVersionPrefix so it special-cases value == "*" and returns ("", "*") (i.e. no prefix, core version "*") so callers get a non-empty version string and can handle the npm wildcard appropriately; update the function extractDependencyVersionPrefix to check for value == "*" before iterating prefixes.
163-174: Revision preservation may carry stale metadata.When the current version has a revision (e.g.,
1.2.3_1) and the latest doesn't (e.g.,1.2.4), the code preserves the old revision, resulting in1.2.4_1. This could be misleading if the revision was specific to the old version (e.g., a distro-specific patch). If this is intentional behavior for your use case, consider adding a comment explaining the rationale.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm.go` around lines 163 - 174, mergeDependencyVersion currently carries the old Semver.Revision from currentVersion into the newer latestVersion.Semver, producing outputs like 1.2.4_1 which may be stale; change the logic in mergeDependencyVersion so you only preserve currentVersion.Semver.Revision when the semantic core (major/minor/patch) actually matches the latest (i.e., the revision applies to the same base version), otherwise do not copy the revision and return latestVersion; update the conditional that checks currentVersion.Semver.HasRevision && !latestVersion.Semver.HasRevision to also verify semver-core equality (or remove the revision-preserving branch entirely) and add a comment explaining the chosen behavior.
🤖 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 69-77: extractDependencyVersionPrefix currently treats a
standalone "*" as a prefix and returns ("*", ""), which leads to an empty core
version and breaks downstream semver parsing; change
extractDependencyVersionPrefix so it special-cases value == "*" and returns ("",
"*") (i.e. no prefix, core version "*") so callers get a non-empty version
string and can handle the npm wildcard appropriately; update the function
extractDependencyVersionPrefix to check for value == "*" before iterating
prefixes.
- Around line 163-174: mergeDependencyVersion currently carries the old
Semver.Revision from currentVersion into the newer latestVersion.Semver,
producing outputs like 1.2.4_1 which may be stale; change the logic in
mergeDependencyVersion so you only preserve currentVersion.Semver.Revision when
the semantic core (major/minor/patch) actually matches the latest (i.e., the
revision applies to the same base version), otherwise do not copy the revision
and return latestVersion; update the conditional that checks
currentVersion.Semver.HasRevision && !latestVersion.Semver.HasRevision to also
verify semver-core equality (or remove the revision-preserving branch entirely)
and add a comment explaining the chosen behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: defa518c-db2b-4134-aa13-8e4f74d02de1
📒 Files selected for processing (4)
npm.gonpm_test.gosemver.gosemver_test.go
…ard support - Avoids carrying revision suffixes across major semver core changes in mergeDependencyVersion. - Adds semverCoreEqual helper to correctly compare version cores. - Improves extractDependencyVersionPrefix to handle wildcard (“*”) versions accurately. - Extends and clarifies tests for wildcard handling, revision logic, and error cases in npm_test.go.
1.2.3_1Closes #17
Summary by CodeRabbit
Release Notes
New Features
Tests