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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds workspace/user ignore lists for package diagnostics and wire-up for adding entries to those lists. New configuration keys for ignore.upgrade, ignore.deprecation, ignore.replacement and ignore.vulnerability are declared and documented. Diagnostic rules for upgrade, deprecation, replacement and vulnerability now consult the corresponding ignore list and skip emitting diagnostics when matched. The quick-fix provider produces additional "add to ignore" actions (workspace and user scopes). Tests and test mocks/setup were updated to include the new configuration shape. Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 2
🧹 Nitpick comments (2)
tests/code-actions/quick-fix.test.ts (2)
35-58: Consider verifying command arguments for ignore actions.The tests verify action titles and counts, but do not assert that the command arguments are correct. Consider adding assertions to verify the
action.command.argumentscontain the expected values (code,ignoreTarget,configTarget). This would catch regressions if the argument order or values change.Example addition for the "vulnerability with fix" test:
expect(actions[1]!.command?.arguments).toEqual([ 'vulnerability', 'lodash@4.17.20', ConfigurationTarget.Workspace, ])
71-84: Add tests for deprecation and replacement ignore actions.The test suite only covers upgrade and vulnerability scenarios. The
addIgnoreRulesfunction insrc/providers/code-actions/quick-fix.tsdefines handlers fordeprecationandreplacementdiagnostic types, but no tests exercise these code paths. Adding test cases for both would improve coverage of the ignore-rule functionality.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/providers/code-actions/index.tssrc/providers/code-actions/quick-fix.tssrc/providers/diagnostics/rules/deprecation.tssrc/providers/diagnostics/rules/replacement.tssrc/providers/diagnostics/rules/vulnerability.tstests/__mocks__/vscode.tstests/__setup__/index.tstests/code-actions/quick-fix.test.ts
| useCommand('npmx.addToIgnore', async (scope: string, name: string, target: ConfigurationTarget) => { | ||
| scope = `ignore.${scope}` | ||
| const config = workspace.getConfiguration(scopedConfigs.scope) | ||
| const current = config.get<string[]>(scope, []) | ||
| if (current.includes(name)) | ||
| return | ||
| await config.update(scope, [...current, name], target) | ||
| }) |
There was a problem hiding this comment.
Validate command arguments before mutating configuration.
npmx.addToIgnore currently trusts runtime arguments. A malformed invocation can write unexpected keys or values into settings.
Suggested patch
useCommand('npmx.addToIgnore', async (scope: string, name: string, target: ConfigurationTarget) => {
+ if (!scope || !name)
+ return
+ if (!['deprecation', 'replacement', 'vulnerability'].includes(scope))
+ return
+
scope = `ignore.${scope}`
const config = workspace.getConfiguration(scopedConfigs.scope)
const current = config.get<string[]>(scope, [])
if (current.includes(name))
return
await config.update(scope, [...current, name], target)
})
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
41-55: Consider adding a short note on ignore-setting precedence.Now that ignore lists exist in multiple scopes, a one-liner on precedence (e.g. project vs workspace vs user) would reduce ambiguity for users.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.mdpackage.jsonsrc/providers/diagnostics/rules/upgrade.tstests/__setup__/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/setup/index.ts
1 participant
Resolve #52