commit 3c69befef00af685fe86b3bc62c36f84f61bb64b Author: Zhongwei Li Date: Sat Nov 29 18:13:50 2025 +0800 Initial commit diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json new file mode 100644 index 0000000..89a2536 --- /dev/null +++ b/.claude-plugin/plugin.json @@ -0,0 +1,12 @@ +{ + "name": "pr-reviewer", + "description": "Reviews pull request changes to provide feedback, check for issues, and suggest improvements before merging into the main codebase.", + "version": "1.0.0", + "author": { + "name": "ClaudeForge Community", + "url": "https://github.com/claudeforge/marketplace" + }, + "commands": [ + "./commands" + ] +} \ No newline at end of file diff --git a/README.md b/README.md new file mode 100644 index 0000000..a95cc34 --- /dev/null +++ b/README.md @@ -0,0 +1,3 @@ +# pr-reviewer + +Reviews pull request changes to provide feedback, check for issues, and suggest improvements before merging into the main codebase. diff --git a/commands/pr-review.md b/commands/pr-review.md new file mode 100644 index 0000000..560bf99 --- /dev/null +++ b/commands/pr-review.md @@ -0,0 +1,344 @@ +--- +description: Review pull requests for code quality, security, bugs, and best practices +version: 1.0.0 +--- + +# PR Reviewer + +Comprehensive pull request review covering code quality, security, testing, and best practices. + +## What It Does + +- Analyzes PR changes for code quality issues +- Identifies potential bugs and edge cases +- Checks security vulnerabilities +- Verifies test coverage +- Suggests improvements and optimizations +- Reviews documentation and naming + +## How to Use + +Provide the PR number to review: + +```bash +/pr-review 789 +``` + +The command will analyze the PR and provide detailed feedback. + +## Review Areas + +**Code Quality** +- Code structure and organization +- Naming conventions +- Error handling +- Code duplication + +**Functionality** +- Logic correctness +- Edge case handling +- Error conditions +- Performance implications + +**Security** +- Input validation +- SQL injection risks +- XSS vulnerabilities +- Authentication/authorization + +**Testing** +- Test coverage +- Test quality +- Missing test cases +- Integration tests + +**Documentation** +- Code comments +- API documentation +- README updates +- Inline explanations + +## Example Review + +**PR #789**: "Add user search feature" + +**Code Quality Issues** +```javascript +// Issue: Missing null check +function searchUsers(query) { + return users.filter(u => u.name.includes(query)); + // Problem: Crashes if query or u.name is null +} + +// Suggestion: +function searchUsers(query) { + if (!query) return []; + return users.filter(u => u.name?.includes(query)); +} +``` + +**Security Concerns** +```javascript +// Issue: SQL injection risk +const query = `SELECT * FROM users WHERE name = '${input}'`; + +// Suggestion: Use parameterized queries +const query = 'SELECT * FROM users WHERE name = ?'; +db.execute(query, [input]); +``` + +**Missing Tests** +```javascript +// Needs tests for: +- Empty search query +- Special characters in query +- Case sensitivity +- No results found +- Null/undefined inputs +``` + +## Use Cases + +- **Pre-Merge Review**: Catch issues before merging to main +- **Learning Tool**: Help team improve code quality +- **Security Audit**: Identify security vulnerabilities +- **Best Practices**: Ensure code follows standards +- **Knowledge Sharing**: Educate on better approaches + +## Best Practices + +- **Be Constructive**: Suggest improvements, don't just criticize +- **Explain Why**: Provide reasoning for suggestions +- **Prioritize Issues**: Mark critical vs nice-to-have changes +- **Test Suggestions**: Verify suggestions actually work +- **Link Resources**: Provide docs or examples when helpful +- **Acknowledge Good Code**: Highlight what's done well + +## Review Checklist + +**Functionality** +- [ ] Code does what PR description claims +- [ ] Edge cases are handled +- [ ] Error conditions are managed +- [ ] No obvious bugs + +**Code Quality** +- [ ] Code is readable and maintainable +- [ ] Functions are focused and single-purpose +- [ ] No code duplication +- [ ] Naming is clear and consistent + +**Security** +- [ ] Input is validated +- [ ] No SQL injection vulnerabilities +- [ ] No XSS vulnerabilities +- [ ] Authentication is required where needed + +**Testing** +- [ ] New code has tests +- [ ] Tests cover edge cases +- [ ] Tests are meaningful +- [ ] All tests pass + +**Performance** +- [ ] No obvious performance issues +- [ ] Database queries are optimized +- [ ] Large data sets are handled efficiently +- [ ] No memory leaks + +**Documentation** +- [ ] Complex logic is commented +- [ ] API changes are documented +- [ ] README is updated if needed +- [ ] Breaking changes are noted + +## Common Issues + +**Missing Error Handling** +```javascript +// Bad +const data = JSON.parse(input); + +// Good +try { + const data = JSON.parse(input); +} catch (error) { + console.error('Invalid JSON:', error); + return null; +} +``` + +**Unsafe Data Access** +```javascript +// Bad +const email = user.profile.email; + +// Good +const email = user?.profile?.email || 'unknown'; +``` + +**Inefficient Loops** +```javascript +// Bad +for (let item of items) { + await processItem(item); +} + +// Good +await Promise.all(items.map(item => processItem(item))); +``` + +**Hardcoded Values** +```javascript +// Bad +const timeout = 5000; + +// Good +const timeout = config.requestTimeout || 5000; +``` + +## Review Comments Format + +**Structure** +```markdown +**Issue**: Brief description of the problem + +**Location**: file.ts:42 + +**Severity**: Critical | High | Medium | Low + +**Suggestion**: +Specific code or approach to fix the issue + +**Reason**: +Why this is important and what could go wrong +``` + +**Example** +```markdown +**Issue**: Potential SQL injection vulnerability + +**Location**: api/users.ts:23 + +**Severity**: Critical + +**Suggestion**: +Use parameterized queries instead of string concatenation: +`db.query('SELECT * FROM users WHERE id = ?', [userId])` + +**Reason**: +Current code allows attackers to inject malicious SQL, +potentially exposing or deleting all user data. +``` + +## Approval Guidelines + +**Approve** if: +- All critical issues are resolved +- Code meets quality standards +- Tests are comprehensive +- No security concerns + +**Request Changes** if: +- Critical bugs exist +- Security vulnerabilities present +- Missing essential tests +- Code quality issues + +**Comment** if: +- Minor improvements suggested +- Questions need clarification +- Architecture discussion needed + +## Testing Review + +Verify tests are: + +**Comprehensive** +```javascript +// Good test coverage +describe('searchUsers', () => { + test('returns matching users', () => { ... }); + test('handles empty query', () => { ... }); + test('is case insensitive', () => { ... }); + test('returns empty array for no matches', () => { ... }); +}); +``` + +**Meaningful** +```javascript +// Good test +expect(result.length).toBe(2); +expect(result[0].name).toBe('Alice'); + +// Bad test +expect(result).toBeTruthy(); // Too vague +``` + +## Performance Review + +Check for: + +**Database Efficiency** +- Are queries optimized? +- Are indexes being used? +- Is there an N+1 query problem? + +**Algorithm Efficiency** +- Could a better algorithm be used? +- Is time complexity acceptable? +- Are there unnecessary iterations? + +**Resource Usage** +- Are large objects being copied unnecessarily? +- Is memory being freed properly? +- Are files/connections closed? + +## Documentation Review + +Ensure: +- Complex logic has comments explaining why +- Public APIs have JSDoc or similar +- Breaking changes are documented +- Migration guides exist if needed + +## Feedback Examples + +**Positive Feedback** +``` +Great job handling edge cases in the validation logic! +The error messages are clear and helpful to users. +``` + +**Constructive Feedback** +``` +Consider extracting this logic into a separate function +to improve readability and make it easier to test. +``` + +**Question** +``` +How does this handle the case when the user is not +authenticated? Should we add a check here? +``` + +## Troubleshooting + +**Too Many Issues**: Focus on critical ones first + +**Unclear Changes**: Ask PR author for clarification + +**Disagreement**: Discuss reasoning, consider team standards + +**Time Constraints**: Do quick pass for critical issues only + +## Quality Standards + +A thorough review includes: +- All code changes examined +- Security implications considered +- Test coverage verified +- Documentation checked +- Performance assessed +- Clear, actionable feedback +- Specific code suggestions where applicable diff --git a/plugin.lock.json b/plugin.lock.json new file mode 100644 index 0000000..f0fe8c8 --- /dev/null +++ b/plugin.lock.json @@ -0,0 +1,45 @@ +{ + "$schema": "internal://schemas/plugin.lock.v1.json", + "pluginId": "gh:claudeforge/marketplace:plugins/commands/pr-reviewer", + "normalized": { + "repo": null, + "ref": "refs/tags/v20251128.0", + "commit": "78ef17742516bc4819720a07c15c7743a59e8f57", + "treeHash": "9eb00ae2f07e4dd64f364b7f29ab80005375f47b2ada18ff0bb7218720698b40", + "generatedAt": "2025-11-28T10:15:35.446912Z", + "toolVersion": "publish_plugins.py@0.2.0" + }, + "origin": { + "remote": "git@github.com:zhongweili/42plugin-data.git", + "branch": "master", + "commit": "aa1497ed0949fd50e99e70d6324a29c5b34f9390", + "repoRoot": "/Users/zhongweili/projects/openmind/42plugin-data" + }, + "manifest": { + "name": "pr-reviewer", + "description": "Reviews pull request changes to provide feedback, check for issues, and suggest improvements before merging into the main codebase.", + "version": "1.0.0" + }, + "content": { + "files": [ + { + "path": "README.md", + "sha256": "dacf266ca6b21f9befe5c256b4b06664e026c66636694ae5abb9d8b5420a4bf1" + }, + { + "path": ".claude-plugin/plugin.json", + "sha256": "29469c52344bcf5fb7c2edddebe54c534746945f4aeb5395cc13616f3c0a2714" + }, + { + "path": "commands/pr-review.md", + "sha256": "307d5c801ff44c0bb618d42486f59d81c62b5fc85d9d8df6b109feb0befe260f" + } + ], + "dirSha256": "9eb00ae2f07e4dd64f364b7f29ab80005375f47b2ada18ff0bb7218720698b40" + }, + "security": { + "scannedAt": null, + "scannerVersion": null, + "flags": [] + } +} \ No newline at end of file