PR Review

---
name: PR Review
description: Comprehensive PR review for oc (OpenShift CLI). Runs build, tests, and linting, then applies Go style improvements and provides detailed code review feedback.
license: Apache-2.0
---

# PR Review

Perform a comprehensive review of pull requests for the oc repository, which is a CLI tool based on kubectl that provides kubectl commands plus OpenShift-specific functionality.

## When to Apply

Use this skill when:
- Reviewing a pull request
- User asks to review code changes
- User requests `/pr-review` or similar commands

## Review Process

Follow these steps in order:

### 1. Dependencies Verification

Ensure Go dependencies are consistent by running:

- `go mod tidy -diff`
  - This command ensures that `go.mod` and `go.sum` are consistent and match the source code in the module.

### 2. Build Verification

Run the build to ensure code compiles:

```bash
make oc
```

- If build fails, report errors and stop the review
- If build succeeds, proceed to testing
- Note: Use `make oc` instead of `make build` to avoid building for all architectures (faster)

### 3. Code Verification

Run verification checks to catch style and potential issues:

```bash
make verify
```

This runs multiple verification targets including:
- `verify-gofmt` - Go formatting checks
- `verify-golint` - Linting checks
- `verify-govet` - Go vet checks
- `verify-cli-conventions` - CLI-specific conventions
- `verify-generated-completions` - Generated code verification

- Report any verification errors or warnings
- Note any patterns that need addressing

### 4. Test Execution

Run the test suite to verify functionality:

```bash
make test
```

- Report any test failures with details
- If critical tests fail, flag for immediate attention
- Proceed even if some tests fail (document them)
- **Known Issue**: Test failure in `github.com/openshift/oc/pkg/cli` (kubeconfig error) can be ignored

### 5. Code Review & Go Style Application

After running the above checks, review the changed code and apply Go best practices.
Start by:

- Load changes against the base branch by using `git diff`.
  The base branch is `main` by default, but it can be overwritten by `[base-git-branch]`
  argument when this skill is invoked using `pr-review` command directly.
- Understand the scope of the changes.

Then proceed to review. Follow these steps:

- **Effective Go Principles**: Apply the Effective Go skill automatically
  - Use `gofmt` for formatting
  - Follow Go naming conventions (MixedCaps/mixedCaps, no underscores)
  - Ensure proper error handling (no ignored errors)
  - Check for idiomatic Go patterns

- **oc-Specific Considerations**:
  - Ensure kubectl compatibility is maintained
  - Verify OpenShift-specific commands follow existing patterns
  - Check that CLI output follows consistent formatting
  - Validate flag definitions match kubectl conventions where applicable

- **Breaking Changes**:
  - Ensure that the command line API is backwards-compatible
    - Check for CLI flag removals or renames
    - Check for changes in command line arguments

- **Code Quality**:
  - Look for potential race conditions
  - Check for resource leaks (unclosed files, connections, goroutine leaks)
    - Goroutine leak patterns to watch:
      - Goroutines without context cancellation handling
      - Missing `select` with `ctx.Done()` case
      - Unbounded channel operations without timeouts
      - `go func()` without proper lifecycle management
      - Use `errgroup` or `sync.WaitGroup` for coordinated goroutines
  - Verify proper context propagation
  - Ensure appropriate logging levels

- **Documentation**:
  - All exported functions/types should have doc comments
  - CLI command help text should be clear and complete
  - Complex logic should have explanatory comments

### 6. Apply Fixes

Based on the review:
- Fix any linting issues automatically where safe
- Apply `gofmt` and `goimports` formatting
- Suggest or implement idiomatic Go improvements
- Document any issues that require manual review

### 7. Summary

Provide a structured summary:
- ✅ Build status
- ✅ Test results (pass/fail counts)
- ✅ Linting status
- 📝 Code quality observations
- 🔧 Changes applied (if any)
- ⚠️  Issues requiring attention

## Key Checks for oc

Since oc is built on kubectl:
- Verify upstream kubectl compatibility
- Check for proper use of kubectl libraries
- Ensure OpenShift-specific features are clearly separated
- Validate that CLI behavior matches kubectl conventions

## References

- [Effective Go](https://go.dev/doc/effective_go)
- [oc Repository](https://github.com/openshift/oc)
- [kubectl Conventions](https://kubernetes.io/docs/reference/kubectl/conventions/)