|
| 1 | +--- |
| 2 | +name: code-review |
| 3 | +description: Automated PR review using comprehensive checklist tailored for modularized Contentstack CLI |
| 4 | +--- |
| 5 | + |
| 6 | +# Code Review Command |
| 7 | + |
| 8 | +## Usage Patterns |
| 9 | + |
| 10 | +### Scope-Based Reviews |
| 11 | +- `/code-review` - Review all current changes with full checklist |
| 12 | +- `/code-review --scope typescript` - Focus on TypeScript configuration and patterns |
| 13 | +- `/code-review --scope testing` - Focus on Mocha/Chai test patterns |
| 14 | +- `/code-review --scope oclif` - Focus on command structure and OCLIF patterns |
| 15 | +- `/code-review --scope packages` - Focus on package structure and organization |
| 16 | + |
| 17 | +### Severity Filtering |
| 18 | +- `/code-review --severity critical` - Show only critical issues (security, breaking changes) |
| 19 | +- `/code-review --severity high` - Show high and critical issues |
| 20 | +- `/code-review --severity all` - Show all issues including suggestions |
| 21 | + |
| 22 | +### Package-Aware Reviews |
| 23 | +- `/code-review --package contentstack-config` - Review changes in specific package |
| 24 | +- `/code-review --package-type plugin` - Review plugin packages only (auth, config) |
| 25 | +- `/code-review --package-type library` - Review library packages (command, utilities, dev-dependencies) |
| 26 | + |
| 27 | +### File Type Focus |
| 28 | +- `/code-review --files commands` - Review command files only |
| 29 | +- `/code-review --files tests` - Review test files only |
| 30 | +- `/code-review --files utils` - Review utility files |
| 31 | + |
| 32 | +## Comprehensive Review Checklist |
| 33 | + |
| 34 | +### Monorepo Structure Compliance |
| 35 | +- **Package organization**: Proper placement in `packages/` structure |
| 36 | +- **pnpm workspace**: Correct `package.json` workspace configuration |
| 37 | +- **Build artifacts**: No `lib/` directories committed to version control |
| 38 | +- **Dependencies**: Proper use of shared utilities (`@contentstack/cli-command`, `@contentstack/cli-utilities`) |
| 39 | +- **Scripts**: Consistent build, test, and lint scripts across packages |
| 40 | + |
| 41 | +### Package-Specific Structure |
| 42 | +- **Plugin packages** (auth, config): Have `oclif.commands` configuration |
| 43 | +- **Library packages** (command, utilities, dev-dependencies): Proper exports in package.json |
| 44 | +- **Main package** (contentstack): Aggregates plugins correctly |
| 45 | +- **Dependency versions**: Using beta versions appropriately (~version ranges) |
| 46 | + |
| 47 | +### TypeScript Standards |
| 48 | +- **Configuration compliance**: Follows package TypeScript config (`strict: false`, `target: es2017`) |
| 49 | +- **Naming conventions**: kebab-case files, PascalCase classes, camelCase functions |
| 50 | +- **Import patterns**: ES modules with proper default/named exports |
| 51 | +- **Type safety**: No unnecessary `any` types in production code |
| 52 | + |
| 53 | +### OCLIF Command Patterns |
| 54 | +- **Base class usage**: Extends `@contentstack/cli-command` Command |
| 55 | +- **Command structure**: Proper `static description`, `static examples`, `static flags` |
| 56 | +- **Topic organization**: Uses `cm` topic structure (`cm:config:set`, `cm:auth:login`) |
| 57 | +- **Error handling**: Uses `handleAndLogError` from utilities |
| 58 | +- **Flag validation**: Early validation and user-friendly error messages |
| 59 | +- **Service delegation**: Commands orchestrate, services implement business logic |
| 60 | + |
| 61 | +### Testing Excellence (Mocha/Chai Stack) |
| 62 | +- **Framework compliance**: Uses Mocha + Chai (not Jest) |
| 63 | +- **File patterns**: Follows `*.test.ts` naming convention |
| 64 | +- **Directory structure**: Proper placement in `test/unit/` |
| 65 | +- **Test organization**: Arrange-Act-Assert pattern consistently used |
| 66 | +- **Isolation**: Proper setup/teardown with beforeEach/afterEach |
| 67 | +- **No real API calls**: All external dependencies properly mocked |
| 68 | + |
| 69 | +### Error Handling Standards |
| 70 | +- **Consistent patterns**: Use `handleAndLogError` from utilities |
| 71 | +- **User-friendly messages**: Clear error descriptions for end users |
| 72 | +- **Logging**: Proper use of `log.debug` for diagnostic information |
| 73 | +- **Status messages**: Use `cliux` for user feedback (success, error, info) |
| 74 | + |
| 75 | +### Build and Compilation |
| 76 | +- **TypeScript compilation**: Clean compilation with no errors |
| 77 | +- **OCLIF manifest**: Generated for command discovery |
| 78 | +- **README generation**: Commands documented in package README |
| 79 | +- **Source maps**: Properly configured for debugging |
| 80 | +- **No build artifacts in commit**: `.gitignore` excludes `lib/` directories |
| 81 | + |
| 82 | +### Testing Coverage |
| 83 | +- **Test structure**: Tests in `test/unit/` with descriptive names |
| 84 | +- **Command testing**: Uses @oclif/test for command validation |
| 85 | +- **Error scenarios**: Tests for both success and failure paths |
| 86 | +- **Mocking**: All dependencies properly mocked |
| 87 | + |
| 88 | +### Package.json Compliance |
| 89 | +- **Correct metadata**: name, description, version, author |
| 90 | +- **Script definitions**: build, compile, test, lint scripts present |
| 91 | +- **Dependencies**: Correct versions of shared packages |
| 92 | +- **Main/types**: Properly configured for library packages |
| 93 | +- **OCLIF config**: Present for plugin packages |
| 94 | + |
| 95 | +### Security and Best Practices |
| 96 | +- **No secrets**: No API keys or tokens in code or tests |
| 97 | +- **Input validation**: Proper validation of user inputs and flags |
| 98 | +- **Process management**: Appropriate use of error codes |
| 99 | +- **File operations**: Safe handling of file system operations |
| 100 | + |
| 101 | +### Code Quality |
| 102 | +- **Naming consistency**: Follow established conventions |
| 103 | +- **Comments**: Only for non-obvious logic (no "narration" comments) |
| 104 | +- **Error messages**: Clear, actionable messages for users |
| 105 | +- **Module organization**: Proper separation of concerns |
| 106 | + |
| 107 | +## Review Execution |
| 108 | + |
| 109 | +### Automated Checks |
| 110 | +1. **Lint compliance**: ESLint checks for code style |
| 111 | +2. **TypeScript compiler**: Successful compilation to `lib/` directories |
| 112 | +3. **Test execution**: All tests pass successfully |
| 113 | +4. **Build verification**: Build scripts complete without errors |
| 114 | + |
| 115 | +### Manual Review Focus Areas |
| 116 | +1. **Command usability**: Clear help text and realistic examples |
| 117 | +2. **Error handling**: Appropriate error messages and recovery options |
| 118 | +3. **Test quality**: Comprehensive test coverage for critical paths |
| 119 | +4. **Monorepo consistency**: Consistent patterns across all packages |
| 120 | +5. **Flag design**: Intuitive flag names and combinations |
| 121 | + |
| 122 | +### Common Issues to Flag |
| 123 | +- **Inconsistent TypeScript settings**: Mixed strict mode without reason |
| 124 | +- **Real API calls in tests**: Unmocked external dependencies |
| 125 | +- **Missing error handling**: Commands that fail silently |
| 126 | +- **Poor test organization**: Tests without clear Arrange-Act-Assert |
| 127 | +- **Build artifacts committed**: `lib/` directories in version control |
| 128 | +- **Unclear error messages**: Non-actionable error descriptions |
| 129 | +- **Inconsistent flag naming**: Similar flags with different names |
| 130 | +- **Missing command examples**: Examples not showing actual usage |
| 131 | + |
| 132 | +## Repository-Specific Checklist |
| 133 | + |
| 134 | +### For Modularized CLI |
| 135 | +- [ ] Command properly extends `@contentstack/cli-command` Command |
| 136 | +- [ ] Flags defined with proper types from `@contentstack/cli-utilities` |
| 137 | +- [ ] Error handling uses `handleAndLogError` utility |
| 138 | +- [ ] User feedback uses `cliux` utilities |
| 139 | +- [ ] Tests use Mocha + Chai pattern with mocked dependencies |
| 140 | +- [ ] Package.json has correct scripts (build, compile, test, lint) |
| 141 | +- [ ] TypeScript compiles with no errors |
| 142 | +- [ ] Tests pass: `pnpm test` |
| 143 | +- [ ] No `.only` or `.skip` in test files |
| 144 | +- [ ] Build succeeds: `pnpm run build` |
| 145 | +- [ ] OCLIF manifest generated successfully |
| 146 | + |
| 147 | +### Before Merge |
| 148 | +- [ ] All review items addressed |
| 149 | +- [ ] No build artifacts in commit |
| 150 | +- [ ] Tests added for new functionality |
| 151 | +- [ ] Documentation updated if needed |
| 152 | +- [ ] No console.log() statements (use log.debug instead) |
| 153 | +- [ ] Error messages are user-friendly |
| 154 | +- [ ] No secrets or credentials in code |
0 commit comments