Sync Gateway Code Review

This skill provides comprehensive code review guidelines for Couchbase Sync Gateway, which manages access and synchronization between Couchbase Lite and Couchbase Server.

What This Skill Does

Performs systematic code reviews following Sync Gateway project standards, focusing on API documentation, logging practices, security, performance, and code quality.

Instructions

When performing a code review for Sync Gateway, follow these steps in order:

1. REST API Documentation Check

If the changes include any modifications to REST APIs:

2. Logging Standards Review

Scan for inappropriate dev-time logging:

- `base.Infof` for informational messages

- `base.Warnf` for warnings

- `base.Debugf` for debug-level messages

- `base.Errorf` for errors

3. PII and User Data Redaction

Review log messages for User Data exposure:

- Document IDs

- JSON document contents (keys and values)

- Usernames

- Email addresses

- Other personally identifiable information (PII)

4. Performance and Concurrency Analysis

Examine code for performance implications:

- Look for locks held for extended periods

- Identify potential deadlock scenarios

- Ensure proper lock ordering

- Check shared state access

- Verify proper synchronization mechanisms

- Look for data races in concurrent operations

- Channel usage patterns

- Goroutine lifecycle management

- Resource cleanup in concurrent code

5. Code Comment Quality

Evaluate comments for clarity and value:

- Why a particular approach was chosen

- What problem the code is solving

- Edge cases or gotchas

- Business logic rationale

6. Loop Safety Review

Inspect all `for` loops for proper termination:

- Loops that depend on external state changes

- Loops without clear termination conditions

- Network or I/O loops without timeouts

Review Checklist Summary

For each pull request, confirm:

Example Usage

**Reviewing a REST API change:**

```

I'll review this PR against Sync Gateway standards:

1. REST API Changes: The new endpoint `/db/_user/{name}` was added

- ❌ OpenAPI spec in docs/api/admin.yaml needs updating with this endpoint

2. Logging: Found `fmt.Printf("User: %s", username)` in handlers/user.go:45

- ❌ Should use `base.Infof("User: %s", base.UD(username))`

- Also wrapping username with base.UD() for PII redaction

3. Concurrency: The userCache mutex is held while making a network call

- ❌ This could cause contention; consider releasing lock before network I/O

4. Loop Safety: Line 120 has `for { ... if condition { break } }`

- ❌ Prefer `for !condition { ... }` to express exit condition clearly

```

Important Notes