11---
22name : code-review
3- description : Automated PR review using comprehensive checklist tailored for Contentstack CLI plugins
3+ description : Automated PR review using comprehensive checklist tailored for modularized Contentstack CLI
44---
55
66# Code Review Command
@@ -10,113 +10,147 @@ description: Automated PR review using comprehensive checklist tailored for Cont
1010### Scope-Based Reviews
1111- ` /code-review ` - Review all current changes with full checklist
1212- ` /code-review --scope typescript ` - Focus on TypeScript configuration and patterns
13- - ` /code-review --scope testing ` - Focus on Mocha/Chai/Sinon test patterns
14- - ` /code-review --scope contentstack ` - Focus on API integration and CLI patterns
13+ - ` /code-review --scope testing ` - Focus on Mocha/Chai test patterns
1514- ` /code-review --scope oclif ` - Focus on command structure and OCLIF patterns
15+ - ` /code-review --scope packages ` - Focus on package structure and organization
1616
1717### Severity Filtering
1818- ` /code-review --severity critical ` - Show only critical issues (security, breaking changes)
1919- ` /code-review --severity high ` - Show high and critical issues
2020- ` /code-review --severity all ` - Show all issues including suggestions
2121
2222### Package-Aware Reviews
23- - ` /code-review --package contentstack-import ` - Review changes in specific package
24- - ` /code-review --package-type plugin ` - Review plugin packages only
25- - ` /code-review --package-type library ` - Review library packages (e.g., variants)
23+ - ` /code-review --package contentstack-import ` - Review changes in import package
24+ - ` /code-review --package contentstack-export ` - Review changes in export package
25+ - ` /code-review --package-type plugin ` - Review all plugin packages (all 11 packages are plugins)
26+ - ` /code-review --package-scope cm ` - Review CM (content management) related packages
2627
2728### File Type Focus
2829- ` /code-review --files commands ` - Review command files only
2930- ` /code-review --files tests ` - Review test files only
30- - ` /code-review --files modules ` - Review import/export modules
31+ - ` /code-review --files utils ` - Review utility files
3132
3233## Comprehensive Review Checklist
3334
3435### Monorepo Structure Compliance
35- - ** Package organization** : Proper placement in ` packages/ ` structure
36- - ** pnpm workspace** : Correct ` package.json ` workspace configuration
36+ - ** Package organization** : 11 plugin packages under ` packages/contentstack-* `
37+ - ** pnpm workspace** : Correct ` pnpm-workspace.yaml ` configuration
3738- ** Build artifacts** : No ` lib/ ` directories committed to version control
3839- ** Dependencies** : Proper use of shared utilities (` @contentstack/cli-command ` , ` @contentstack/cli-utilities ` )
40+ - ** Scripts** : Consistent build, test, and lint scripts across packages
3941
40- ### TypeScript Standards (Repository-Specific)
41- - ** Configuration compliance** : Follows package-specific TypeScript config
42+ ### Package-Specific Structure
43+ - ** All packages are plugins** : Each has ` oclif.commands ` configuration pointing to ` ./lib/commands `
44+ - ** Plugin topics** : All commands under ` cm: ` topic (content management)
45+ - ** Base commands** : Each plugin defines its own ` BaseCommand ` extending ` @contentstack/cli-command ` Command
46+ - ** Inter-plugin dependencies** : Some plugins depend on others (e.g., import depends on audit)
47+ - ** Dependency versions** : Using consistent versions across plugins
48+
49+ ### TypeScript Standards
50+ - ** Configuration compliance** : Follows package TypeScript config (` strict: false ` , ` target: es2017 ` )
4251- ** Naming conventions** : kebab-case files, PascalCase classes, camelCase functions
43- - ** Type safety** : Appropriate use of strict mode vs relaxed settings per package
4452- ** Import patterns** : ES modules with proper default/named exports
45- - ** Migration strategy** : Proper use of ` @ts-ignore ` during gradual migration
46-
47- ### OCLIF Command Patterns (Actual Implementation)
48- - ** Base class usage** : Extends ` @contentstack/cli-command ` (not ` @oclif/core ` )
49- - ** Command structure** : Proper ` static description ` , ` examples ` , ` flags `
50- - ** Topic organization** : Uses ` cm ` topic structure (` cm:stacks:import ` )
51- - ** Error handling** : Uses ` handleAndLogError ` from utilities
52- - ** Validation** : Early flag validation and user-friendly error messages
53- - ** Service delegation** : Commands orchestrate, services implement business logic
54-
55- ### Testing Excellence (Mocha/Chai/Sinon Stack)
56- - ** Framework compliance** : Uses Mocha + Chai + Sinon (not Jest)
57- - ** File patterns** : Follows ` *.test.ts ` naming (or ` *.test.js ` for bootstrap)
58- - ** Directory structure** : Proper placement in ` test/unit/ ` , ` test/lib/ ` , etc.
59- - ** Mock patterns** : Proper Sinon stubbing of SDK methods
60- - ** Coverage configuration** : Correct nyc setup (watch for ` inlcude ` typo)
61- - ** Test isolation** : Proper ` beforeEach ` /` afterEach ` with ` sinon.restore() `
53+ - ** Type safety** : No unnecessary ` any ` types in production code
54+
55+ ### OCLIF Command Patterns
56+ - ** Base class usage** : Extends plugin-specific ` BaseCommand ` or ` @contentstack/cli-command ` Command
57+ - ** Command structure** : Proper ` static id ` , ` static description ` , ` static examples ` , ` static flags `
58+ - ** Topic organization** : Uses ` cm:stacks:* ` structure (` cm:stacks:import ` , ` cm:stacks:export ` , ` cm:stacks:audit ` )
59+ - ** Error handling** : Uses ` handleAndLogError ` from utilities with context
60+ - ** Flag validation** : Early validation and user-friendly error messages
61+ - ** Service delegation** : Commands are thin, services handle business logic
62+
63+ ### Testing Excellence (Mocha/Chai Stack)
64+ - ** Framework compliance** : Uses Mocha + Chai (not Jest)
65+ - ** File patterns** : Follows ` *.test.ts ` naming convention
66+ - ** Directory structure** : Proper placement in ` test/unit/ `
67+ - ** Test organization** : Arrange-Act-Assert pattern consistently used
68+ - ** Isolation** : Proper setup/teardown with beforeEach/afterEach
6269- ** No real API calls** : All external dependencies properly mocked
6370
64- ### Contentstack API Integration (Real Patterns)
65- - ** SDK usage** : Proper ` managementSDKClient ` and fluent API chaining
66- - ** Authentication** : Correct ` configHandler ` and token alias handling
67- - ** Rate limiting compliance** :
68- - Batch spacing (minimum 1 second between batches)
69- - 429 retry handling with exponential backoff
70- - Pagination throttling for variants
71- - ** Error handling** : Proper ` handleAndLogError ` usage and user-friendly messages
72- - ** Configuration** : Proper regional endpoint and management token handling
73-
74- ### Import/Export Module Architecture
75- - ** BaseClass extension** : Proper inheritance from import/export BaseClass
76- - ** Batch processing** : Correct use of ` makeConcurrentCall ` and ` logMsgAndWaitIfRequired `
77- - ** Module organization** : Proper entity-specific module structure
78- - ** Configuration handling** : Proper ` ModuleClassParams ` usage
79- - ** Progress feedback** : Appropriate user feedback during long operations
71+ ### Error Handling Standards
72+ - ** Consistent patterns** : Use ` handleAndLogError ` from utilities
73+ - ** User-friendly messages** : Clear error descriptions for end users
74+ - ** Logging** : Proper use of ` log.debug ` for diagnostic information
75+ - ** Status messages** : Use ` cliux ` for user feedback (success, error, info)
76+
77+ ### Build and Compilation
78+ - ** TypeScript compilation** : Clean compilation with no errors
79+ - ** OCLIF manifest** : Generated for command discovery
80+ - ** README generation** : Commands documented in package README
81+ - ** Source maps** : Properly configured for debugging
82+ - ** No build artifacts in commit** : ` .gitignore ` excludes ` lib/ ` directories
83+
84+ ### Testing Coverage
85+ - ** Test structure** : Tests in ` test/unit/ ` with descriptive names
86+ - ** Command testing** : Uses @oclif/test for command validation
87+ - ** Error scenarios** : Tests for both success and failure paths
88+ - ** Mocking** : All dependencies properly mocked
89+
90+ ### Package.json Compliance
91+ - ** Correct metadata** : name, description, version, author
92+ - ** Script definitions** : build, compile, test, lint scripts present
93+ - ** Dependencies** : Correct versions of shared packages
94+ - ** Main/types** : Properly configured for library packages
95+ - ** OCLIF config** : Present for plugin packages
8096
8197### Security and Best Practices
82- - ** Token security ** : No API keys or tokens logged or committed
98+ - ** No secrets ** : No API keys or tokens in code or tests
8399- ** Input validation** : Proper validation of user inputs and flags
84- - ** Error exposure** : No sensitive information in error messages
85- - ** File permissions** : Proper handling of file system operations
86- - ** Process management** : Appropriate use of ` process.exit(1) ` for critical failures
87-
88- ### Performance Considerations
89- - ** Concurrent processing** : Proper use of ` Promise.allSettled ` for batch operations
90- - ** Memory management** : Appropriate handling of large datasets
91- - ** Rate limiting** : Compliance with Contentstack API limits (10 req/sec)
92- - ** Batch sizing** : Appropriate batch sizes for different operations
93- - ** Progress tracking** : Efficient progress reporting without performance impact
94-
95- ### Package-Specific Patterns
96- - ** Plugin vs Library** : Correct ` oclif.commands ` configuration for plugin packages
97- - ** Command compilation** : Proper build pipeline (` tsc ` → ` lib/commands ` → ` oclif manifest ` )
98- - ** Dependency management** : Correct use of shared vs package-specific dependencies
99- - ** Test variations** : Handles different test patterns per package (JS vs TS, different structures)
100+ - ** Process management** : Appropriate use of error codes
101+ - ** File operations** : Safe handling of file system operations
102+
103+ ### Code Quality
104+ - ** Naming consistency** : Follow established conventions
105+ - ** Comments** : Only for non-obvious logic (no "narration" comments)
106+ - ** Error messages** : Clear, actionable messages for users
107+ - ** Module organization** : Proper separation of concerns
100108
101109## Review Execution
102110
103111### Automated Checks
104- 1 . ** Lint compliance** : ESLint and TypeScript compiler checks
105- 2 . ** Test coverage ** : nyc coverage thresholds (where enforced)
106- 3 . ** Build verification ** : Successful compilation to ` lib/ ` directories
107- 4 . ** Dependency audit ** : No security vulnerabilities in dependencies
112+ 1 . ** Lint compliance** : ESLint checks for code style
113+ 2 . ** TypeScript compiler ** : Successful compilation to ` lib/ ` directories
114+ 3 . ** Test execution ** : All tests pass successfully
115+ 4 . ** Build verification ** : Build scripts complete without errors
108116
109117### Manual Review Focus Areas
110- 1 . ** API integration patterns ** : Verify proper SDK usage and error handling
111- 2 . ** Rate limiting implementation ** : Check for proper throttling mechanisms
112- 3 . ** Test quality** : Verify comprehensive mocking and error scenario coverage
113- 4 . ** Command usability ** : Ensure clear help text and examples
114- 5 . ** Monorepo consistency ** : Check for consistent patterns across packages
118+ 1 . ** Command usability ** : Clear help text and realistic examples
119+ 2 . ** Error handling ** : Appropriate error messages and recovery options
120+ 3 . ** Test quality** : Comprehensive test coverage for critical paths
121+ 4 . ** Monorepo consistency ** : Consistent patterns across all packages
122+ 5 . ** Flag design ** : Intuitive flag names and combinations
115123
116124### Common Issues to Flag
117- - ** Coverage config typos** : ` "inlcude" ` instead of ` "include" ` in ` .nycrc.json `
118- - ** Inconsistent TypeScript** : Mixed strict mode usage without migration plan
119- - ** Real API calls in tests** : Any unmocked external dependencies
120- - ** Missing rate limiting** : API calls without proper throttling
121- - ** Build artifacts committed** : Any ` lib/ ` directories in version control
122- - ** Inconsistent error handling** : Not using utilities error handling patterns
125+ - ** Inconsistent TypeScript settings** : Mixed strict mode without reason
126+ - ** Real API calls in tests** : Unmocked external dependencies
127+ - ** Missing error handling** : Commands that fail silently
128+ - ** Poor test organization** : Tests without clear Arrange-Act-Assert
129+ - ** Build artifacts committed** : ` lib/ ` directories in version control
130+ - ** Unclear error messages** : Non-actionable error descriptions
131+ - ** Inconsistent flag naming** : Similar flags with different names
132+ - ** Missing command examples** : Examples not showing actual usage
133+
134+ ## Repository-Specific Checklist
135+
136+ ### For Modularized CLI
137+ - [ ] Command properly extends ` @contentstack/cli-command ` Command
138+ - [ ] Flags defined with proper types from ` @contentstack/cli-utilities `
139+ - [ ] Error handling uses ` handleAndLogError ` utility
140+ - [ ] User feedback uses ` cliux ` utilities
141+ - [ ] Tests use Mocha + Chai pattern with mocked dependencies
142+ - [ ] Package.json has correct scripts (build, compile, test, lint)
143+ - [ ] TypeScript compiles with no errors
144+ - [ ] Tests pass: ` pnpm test `
145+ - [ ] No ` .only ` or ` .skip ` in test files
146+ - [ ] Build succeeds: ` pnpm run build `
147+ - [ ] OCLIF manifest generated successfully
148+
149+ ### Before Merge
150+ - [ ] All review items addressed
151+ - [ ] No build artifacts in commit
152+ - [ ] Tests added for new functionality
153+ - [ ] Documentation updated if needed
154+ - [ ] No console.log() statements (use log.debug instead)
155+ - [ ] Error messages are user-friendly
156+ - [ ] No secrets or credentials in code
0 commit comments