|
| 1 | +# Standard PR Review Checklist |
| 2 | + |
| 3 | +## PR Meta Information |
| 4 | + |
| 5 | +- **PR Title**: Ensure the PR title is clear, concise, and adheres to the project's naming standards (e.g., `[Feature] Add X`, `[Bugfix] Fix Y`, `[Refactor] Z`). |
| 6 | +- **Description**: Include a detailed description explaining: |
| 7 | + - What the PR does. |
| 8 | + - Why the changes are necessary. |
| 9 | + - Links to related issues, tickets, or tasks (if applicable). |
| 10 | +- **Checklist in Description (Optional)**: Include a checklist of tasks completed (if the project uses task checklists). |
| 11 | +- **Reviewer Assignment**: The PR must be assigned to at least one reviewer. |
| 12 | +- **Labels/Tags (Optional)**: Ensure the PR has appropriate labels/tags (e.g., `Bugfix`, `Feature`, `Documentation`). |
| 13 | +- **Branch Naming**: Follow the project's branch naming conventions (e.g., `feature/xyz`, `bugfix/abc`). |
| 14 | + |
| 15 | +## Code Changes |
| 16 | + |
| 17 | +- **Scope of Changes**: |
| 18 | + - PR should contain changes related to only one feature, bugfix, or task. |
| 19 | + - Avoid mixing multiple unrelated changes in one PR. |
| 20 | + - PRs should not exceed a maximum of 500 lines of code (excluding tests and documentation). |
| 21 | +- **Clean Code**: |
| 22 | + - Code should follow the project's coding standards (naming conventions, indentation, spacing). |
| 23 | + - Ensure proper formatting (auto-format code if tools are available). |
| 24 | +- **No Repetition**: Avoid duplicate code; refactor into reusable functions or components where possible. |
| 25 | + - **No Hardcoded Values**: Replace hardcoded values with constants, environment variables, or configurations. |
| 26 | +- **Breaking Down Large Changes**: |
| 27 | + - For large features, break down changes into smaller, manageable PRs. |
| 28 | + |
| 29 | +## Commit Messages |
| 30 | + |
| 31 | +- **Format**: Ensure commit messages follow the [company's git standards](https://github.com/OsmosysSoftware/dev-standards/blob/main/coding-standards/git.md). |
| 32 | + - Example: `[Component] Short description of change` |
| 33 | + - Use imperative mood (e.g., "Fix bug" instead of "Fixed bug"). |
| 34 | + - Provide detailed explanations if the change is complex. |
| 35 | + |
| 36 | +## Quality and Best Practices |
| 37 | + |
| 38 | +- **Tests**: |
| 39 | + - Ensure unit tests or integration tests are added/updated for the feature or bugfix. |
| 40 | + - Run all tests and confirm they pass locally. |
| 41 | + - Include both positive and negative test cases. |
| 42 | +- **Logging**: |
| 43 | + - Follow project's logging standards. |
| 44 | + - Include appropriate log levels (DEBUG, INFO, ERROR). |
| 45 | + - Avoid sensitive information in logs. |
| 46 | +- **Linting and Static Analysis**: |
| 47 | + - Code should pass all linting checks (e.g., ESLint, Flake8, etc.). |
| 48 | + - Resolve any warnings or errors flagged by static analysis tools. |
| 49 | +- **Error Handling**: Ensure proper error handling is implemented (avoid swallowing errors silently). |
| 50 | +- **Performance Considerations**: Check for code efficiency (e.g., avoid unnecessary loops, database queries, or API calls). |
| 51 | + |
| 52 | +## Compliance |
| 53 | + |
| 54 | +- **CI/CD Pipelines**: |
| 55 | + - Ensure all CI pipelines (builds, tests, deployments) pass successfully. |
| 56 | + - Attach a CI screenshot if the pipelines aren't integrated into the PR system. |
| 57 | +- **Security**: |
| 58 | + - Verify no sensitive information (API keys, passwords, etc.) is exposed in the code or configuration. |
| 59 | + - Check for usage of secure methods and libraries. |
| 60 | +- **Backward Compatibility**: Ensure the changes don't break existing functionality or introduce regressions. |
| 61 | + |
| 62 | +## Documentation and UI |
| 63 | + |
| 64 | +- **Documentation**: Update relevant documentation (e.g., README, API documentation, comments) for new features or changes. |
| 65 | +- **Screenshots/Videos (Optional)**: |
| 66 | + - Add screenshots or video recordings for UI changes (if applicable). |
| 67 | + - Ensure the UI matches the design and is responsive on different screen sizes. |
| 68 | + |
| 69 | +## Miscellaneous |
| 70 | + |
| 71 | +- **Dead Code**: Remove any unused code, variables, or imports. |
| 72 | +- **Debugging Artifacts**: Ensure no debugging code (e.g., `console.log`, `print`) is left in the codebase. |
| 73 | +- **Dependencies**: |
| 74 | + - Verify no unnecessary or outdated dependencies are introduced. |
| 75 | + - Lock file changes (e.g., `package-lock.json`, `requirements.txt`) should be reviewed. |
| 76 | + - Check for security vulnerabilities in new dependencies. |
| 77 | + - Ensure compatibility with existing dependencies. |
| 78 | + |
| 79 | +## Final Checks |
| 80 | + |
| 81 | +- **Self-Review**: The developer submitting the PR must perform a thorough self-review before assigning it to a reviewer. |
| 82 | +- **Cross-Environment Testing (Optional)**: Confirm the changes work in multiple environments (e.g., dev, staging, production). |
| 83 | +- **Feature Flags (Optional)**: Ensure new features are behind feature flags if they are not fully ready for release. |
| 84 | + |
0 commit comments