218 lines
6.3 KiB
Markdown
218 lines
6.3 KiB
Markdown
---
|
|
name: go-pr-review
|
|
description: Reviews pull requests in Go repositories with comprehensive checks including code quality, tests, linting, type checking, and Makefile validation
|
|
---
|
|
|
|
# Go Pull Request Review
|
|
|
|
This skill performs comprehensive reviews of pull requests in Go repositories, with special attention to projects
|
|
using Makefiles for build automation.
|
|
|
|
## When to Use This Skill
|
|
|
|
Use this skill when:
|
|
- A developer requests a PR review for a Go repository
|
|
- You need to validate changes before merging
|
|
- You want to ensure code quality, test coverage, and build success
|
|
- The repository uses a Makefile for build automation
|
|
|
|
## Review Process
|
|
|
|
### 0. Validate Repository Compatibility
|
|
|
|
**IMPORTANT: Before proceeding, verify this is a compatible Go repository.**
|
|
|
|
Check for required characteristics:
|
|
```bash
|
|
# 1. Must be a Go repository
|
|
test -f go.mod || echo "ERROR: Not a Go repository (no go.mod found)"
|
|
|
|
# 2. Must have a Makefile
|
|
test -f Makefile || echo "ERROR: No Makefile found"
|
|
|
|
# 3. Check Makefile has test/lint targets
|
|
grep -E "^(test|lint|tidy):" Makefile || echo "WARNING: Makefile missing common targets"
|
|
|
|
Abort conditions:
|
|
|
|
• ❌ No go.mod file exists → This is not a Go repository
|
|
• ❌ No Makefile exists → This skill requires Makefile-based workflows
|
|
• ⚠️ Makefile missing test/lint targets → Proceed with caution, manual validation required
|
|
|
|
If repository is incompatible: Stop execution and inform the user: "This repository doesn't match the requirements
|
|
for the go-pr-review skill (requires Go with Makefile). Would you like a standard code review instead?"
|
|
|
|
### 1. Gather PR Information
|
|
|
|
Fetch the PR details using the gh CLI:
|
|
|
|
• Get the PR diff and changed files
|
|
• Identify the PR number and branch
|
|
• Review the PR description and context
|
|
|
|
### 2. Analyze Changed Files
|
|
|
|
Review all modified files with focus on:
|
|
|
|
• Go code changes: Logic, patterns, error handling
|
|
• Test files: Coverage of new functionality
|
|
• Documentation: README, AGENTS.md, comments (check for AIDEV- anchors)
|
|
• Configuration: go.mod, Makefile, YAML files
|
|
|
|
### 3. Check Makefile Commands
|
|
|
|
Read the Makefile to identify available validation commands. Common commands include:
|
|
|
|
• make lint - Run linters (golangci-lint)
|
|
• make tidy - Format and tidy code
|
|
• make test-api - Run integration/API tests (most critical for validating)
|
|
|
|
### 4. Run Validation Commands
|
|
|
|
# Run tests and linting
|
|
make lint && make tidy && make test-api
|
|
|
|
Check for:
|
|
|
|
• Test failures
|
|
• Linting errors or warnings
|
|
• Build failures
|
|
• Documentation generation errors
|
|
|
|
### 5. Code Quality Review
|
|
|
|
Review code against project standards from AGENTS.md (if exists) and references/go-standards.md:
|
|
|
|
Architecture & Design
|
|
|
|
• Does the change follow existing patterns?
|
|
• Is the business logic in the correct layer? (handlers → service → API clients)
|
|
• Are there any breaking changes to API contracts?
|
|
|
|
Go Best Practices
|
|
|
|
• Proper error handling (don't ignore errors)
|
|
• Context propagation for cancellation
|
|
• Resource cleanup (defer statements)
|
|
• Concurrency safety (mutexes, channels)
|
|
• No hardcoded values (use constants or env vars)
|
|
|
|
Code Style
|
|
|
|
• Follows project conventions (check neighboring files)
|
|
• No unnecessary comments (unless documenting complex business logic)
|
|
• Lowercase SQL keywords (e.g., select not SELECT) if applicable
|
|
• Proper use of existing libraries (check go.mod)
|
|
|
|
API Documentation (Swag) - If repository uses Swag
|
|
|
|
• All HTTP handlers have complete Swag comments
|
|
• Required annotations: @Summary, @Description, @Tags, @Router
|
|
• All parameters documented with @Param
|
|
• All response codes documented (@Success, @Failure)
|
|
• Model references are correct
|
|
|
|
Security
|
|
|
|
• No secrets or credentials in code
|
|
• Input validation on all user inputs
|
|
• Proper authentication/authorization checks
|
|
• No SQL injection risks (use parameterized queries/stored procedures)
|
|
|
|
Testing
|
|
|
|
• New functionality has test coverage
|
|
• Tests are meaningful, not just for coverage numbers
|
|
• Integration tests for API endpoints
|
|
• Edge cases are considered
|
|
|
|
### 6. Review Anchor Comments
|
|
|
|
Search for developer anchor comments:
|
|
|
|
rg "AIDEV-(NOTE|TODO|QUESTION):" --type go
|
|
|
|
These indicate areas an agent flagged for review. Assess whether:
|
|
|
|
• The concern is valid
|
|
• The implementation needs improvement
|
|
|
|
Typically, the anchor should be removed before merge
|
|
|
|
### 7. Provide Structured Feedback
|
|
|
|
Organize findings into categories:
|
|
|
|
Critical Issues (must fix before merge)
|
|
|
|
• Test failures
|
|
• Build/lint errors
|
|
• Security vulnerabilities
|
|
• Breaking changes without versioning
|
|
|
|
Suggestions (should consider)
|
|
|
|
• Code quality improvements
|
|
• Better error messages
|
|
• Performance optimizations
|
|
• Missing edge case tests
|
|
|
|
Observations (nice to have)
|
|
|
|
• Refactoring opportunities
|
|
• Documentation enhancements
|
|
• Code organization improvements
|
|
|
|
### 8. Summary and Recommendation
|
|
|
|
Provide a clear verdict:
|
|
|
|
• ✅ Approved: Ready to merge
|
|
• ⚠️ Approved with suggestions: Can merge, but consider improvements
|
|
• ❌ Request changes: Must address critical issues
|
|
|
|
## Example Review Workflow
|
|
|
|
# 0. Validate repository compatibility
|
|
test -f go.mod && test -f Makefile && echo "Repository compatible" || echo "Repository incompatible - ABORT"
|
|
|
|
# 1. Get PR information
|
|
gh pr view 123 --json number,title,body,changedFiles
|
|
|
|
# 2. Get the diff
|
|
gh pr diff 123
|
|
|
|
# 3. Check Makefile for available commands
|
|
cat Makefile
|
|
|
|
# 4. Run validations (adjust based on Makefile)
|
|
make test-unit
|
|
make test-api
|
|
make lint
|
|
make tidy
|
|
make docs
|
|
|
|
# 5. Search for anchor comments
|
|
rg "AIDEV-" --type go
|
|
|
|
# 6. Review changed files
|
|
# (Use Read tool for each changed file)
|
|
|
|
## Reference Materials
|
|
|
|
See references/go-standards.md for detailed Go code quality standards and common issues to watch for.
|
|
|
|
## Important Notes
|
|
|
|
• Validate repository first - Don't waste time on incompatible repos
|
|
• Never modify test files unless explicitly requested
|
|
• Never change API contracts - version endpoints instead
|
|
• Never alter migration files - data loss risk
|
|
• Always verify tests pass before approving
|
|
• Check that make commands succeed - this is critical validation
|
|
• Look for AIDEV- comments - they highlight areas needing attention
|
|
• Validate Swag comments for all new/modified handlers (if applicable)
|
|
|
|
Remember: The goal is constructive feedback that improves code quality while respecting the developer's intent and
|
|
design decisions.
|