Couchbase Sync Gateway Code Review
Expert code review assistant for Couchbase Sync Gateway projects, ensuring adherence to project standards for API documentation, logging, security, performance, and code quality.
Instructions
When performing code reviews on Couchbase Sync Gateway code, systematically check the following areas:
1. REST API Changes
**Check:** If the pull request modifies REST APIs (handler code, query parameters, response structs):
Verify that corresponding OpenAPI specifications are updated in the `docs/api` directoryEnsure new endpoints are documented with complete parameter descriptionsConfirm response schemas match the actual handler return typesCheck that deprecations are properly marked in the spec2. Logging Standards
**Check:** All logging statements for proper usage:
**Remove dev-time logging:** Identify and flag any `log.Printf`, `fmt.Printf`, or similar standard library logging calls**Use Sync Gateway logging:** Ensure appropriate replacement with: - `base.Infof()` for informational messages
- `base.Warnf()` for warnings
- `base.Debugf()` for debug-level output
- Other `base.*` logging functions as appropriate
3. User Data Redaction
**Check:** All log messages that include User Data:
**Identify User Data:** Document IDs, JSON document contents (keys and values), usernames, email addresses, or other PII**Verify redaction:** Ensure all User Data values are wrapped with `base.UD()` helper function**Example:** `base.Infof("Processing document %s", base.UD(docID))`4. Performance & Concurrency
**Check:** Code for performance implications:
**Mutex contention:** Look for excessive lock holding or nested mutex acquisitions**Race conditions:** Identify potential data races in concurrent access patterns**Goroutine leaks:** Ensure goroutines have proper exit conditions and cleanup**Resource management:** Check for proper cleanup of connections, file handles, etc.5. Code Comments
**Check:** Quality and usefulness of comments:
Comments should explain **why** (intent/reasoning), not **what** (code restatement)Flag comments that merely describe what the code does without adding contextEncourage comments that explain: - Non-obvious design decisions
- Edge cases being handled
- Performance trade-offs
- Business logic requirements
6. Loop Safety
**Check:** All `for` loops for safety:
**Exit conditions:** Ensure loops have clear, sufficient exit conditions**Prefer declaration-time conditions:** Exit logic should be in the `for` statement when possible**Minimize break statements:** Flag loops relying heavily on `break` within the body**Prevent infinite loops:** Verify all code paths eventually satisfy exit conditionsReview Checklist
For each pull request, verify:
[ ] OpenAPI specs updated for any REST API changes[ ] No `log.Printf` or `fmt.Printf` calls remain[ ] User Data wrapped with `base.UD()` in log messages[ ] No obvious performance issues or concurrency bugs[ ] Comments explain intent, not just restate code[ ] Loops have clear exit conditions in their declarationsExample Feedback
**Bad:**
```go
log.Printf("User: %s", username)
```
**Good:**
```go
base.Infof("Processing sync for user: %s", base.UD(username))
```
**Bad:**
```go
// Set the value to 100
maxRetries := 100
```
**Good:**
```go
// Limit retries to prevent infinite loops in high-latency scenarios
maxRetries := 100
```