From e0f43290c5843e2f32e6a1a93495bdc1009b209b Mon Sep 17 00:00:00 2001 From: Jatin Gupta Date: Thu, 23 Jan 2025 09:10:39 +0530 Subject: [PATCH 1/6] Add PR review checklist --- .../standared-PR-review-checklist.md | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 tech-standards/standared-PR-review-checklist.md diff --git a/tech-standards/standared-PR-review-checklist.md b/tech-standards/standared-PR-review-checklist.md new file mode 100644 index 0000000..13bc571 --- /dev/null +++ b/tech-standards/standared-PR-review-checklist.md @@ -0,0 +1,65 @@ +# Standard PR Review Checklist + +## PR Meta Information + +- **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`). +- **Description**: Include a detailed description explaining: + - What the PR does. + - Why the changes are necessary. + - Links to related issues, tickets, or tasks (if applicable). +- **Checklist in Description (Optional)**: Include a checklist of tasks completed (if the project uses task checklists). +- **Reviewer Assignment**: The PR must be assigned to at least one reviewer. +- **Labels/Tags (Optional)**: Ensure the PR has appropriate labels/tags (e.g., `Bugfix`, `Feature`, `Documentation`). + +## Code Changes + +- **Scope of Changes**: + - PR should contain changes related to only one feature, bugfix, or task. + - Avoid mixing multiple unrelated changes in one PR. +- **Clean Code**: + - Code should follow the project's coding standards (naming conventions, indentation, spacing). + - Ensure proper formatting (auto-format code if tools are available). +- **No Repetition**: Avoid duplicate code; refactor into reusable functions or components where possible. +- **No Hardcoded Values**: Replace hardcoded values with constants, environment variables, or configurations. + +## Quality and Best Practices + +- **Tests**: + - Ensure unit tests or integration tests are added/updated for the feature or bugfix. + - Run all tests and confirm they pass locally. +- **Linting and Static Analysis**: + - Code should pass all linting checks (e.g., ESLint, Flake8, etc.). + - Resolve any warnings or errors flagged by static analysis tools. +- **Error Handling**: Ensure proper error handling is implemented (avoid swallowing errors silently). +- **Performance Considerations**: Check for code efficiency (e.g., avoid unnecessary loops, database queries, or API calls). + +## Compliance + +- **CI/CD Pipelines**: + - Ensure all CI pipelines (builds, tests, deployments) pass successfully. + - Attach a CI screenshot if the pipelines aren't integrated into the PR system. +- **Security**: + - Verify no sensitive information (API keys, passwords, etc.) is exposed in the code or configuration. + - Check for usage of secure methods and libraries. +- **Backward Compatibility**: Ensure the changes don't break existing functionality or introduce regressions. + +## Documentation and UI + +- **Documentation (Optional)**: Update relevant documentation (e.g., README, API documentation, comments) for new features or changes. +- **Screenshots/Videos (Optional)**: + - Add screenshots or video recordings for UI changes (if applicable). + - Ensure the UI matches the design and is responsive on different screen sizes. + +## Miscellaneous + +- **Dead Code**: Remove any unused code, variables, or imports. +- **Debugging Artifacts**: Ensure no debugging code (e.g., `console.log`, `print`) is left in the codebase. +- **Dependencies (Optional)**: + - Verify no unnecessary or outdated dependencies are introduced. + - Lock file changes (e.g., `package-lock.json`, `requirements.txt`) should be reviewed. + +## Final Checks + +- **Self-Review**: The developer submitting the PR must perform a thorough self-review before assigning it to a reviewer. +- **Cross-Environment Testing (Optional)**: Confirm the changes work in multiple environments (e.g., dev, staging, production). +- **Feature Flags (Optional)**: Ensure new features are behind feature flags if they are not fully ready for release. From df5c05d675b362f5ccc56cb400b99e261a598a7b Mon Sep 17 00:00:00 2001 From: Jatin Gupta Date: Thu, 23 Jan 2025 09:20:15 +0530 Subject: [PATCH 2/6] Rename the file --- ...red-PR-review-checklist.md => standard-PR-review-checklist.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tech-standards/{standared-PR-review-checklist.md => standard-PR-review-checklist.md} (100%) diff --git a/tech-standards/standared-PR-review-checklist.md b/tech-standards/standard-PR-review-checklist.md similarity index 100% rename from tech-standards/standared-PR-review-checklist.md rename to tech-standards/standard-PR-review-checklist.md From f73cb46b39d98b68b4eeccb00572887236c77ca3 Mon Sep 17 00:00:00 2001 From: Jatin Gupta Date: Thu, 23 Jan 2025 09:34:49 +0530 Subject: [PATCH 3/6] Update the checklist --- tech-standards/standard-PR-review-checklist.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tech-standards/standard-PR-review-checklist.md b/tech-standards/standard-PR-review-checklist.md index 13bc571..1ded1be 100644 --- a/tech-standards/standard-PR-review-checklist.md +++ b/tech-standards/standard-PR-review-checklist.md @@ -10,17 +10,28 @@ - **Checklist in Description (Optional)**: Include a checklist of tasks completed (if the project uses task checklists). - **Reviewer Assignment**: The PR must be assigned to at least one reviewer. - **Labels/Tags (Optional)**: Ensure the PR has appropriate labels/tags (e.g., `Bugfix`, `Feature`, `Documentation`). +- **Branch Naming**: Follow the project's branch naming conventions (e.g., `feature/xyz`, `bugfix/abc`). ## Code Changes - **Scope of Changes**: - PR should contain changes related to only one feature, bugfix, or task. - Avoid mixing multiple unrelated changes in one PR. + - PRs should not exceed a maximum of 500 lines of code (excluding tests and documentation). - **Clean Code**: - Code should follow the project's coding standards (naming conventions, indentation, spacing). - Ensure proper formatting (auto-format code if tools are available). - **No Repetition**: Avoid duplicate code; refactor into reusable functions or components where possible. -- **No Hardcoded Values**: Replace hardcoded values with constants, environment variables, or configurations. + - **No Hardcoded Values**: Replace hardcoded values with constants, environment variables, or configurations. +- **Breaking Down Large Changes**: + - For large features, break down changes into smaller, manageable PRs. + +## Commit Messages + +- **Format**: Ensure commit messages follow the [company's git standards](https://github.com/OsmosysSoftware/dev-standards/blob/main/coding-standards/git.md). + - Example: `[Component] Short description of change` + - Use imperative mood (e.g., "Fix bug" instead of "Fixed bug"). + - Provide detailed explanations if the change is complex. ## Quality and Best Practices @@ -45,7 +56,7 @@ ## Documentation and UI -- **Documentation (Optional)**: Update relevant documentation (e.g., README, API documentation, comments) for new features or changes. +- **Documentation**: Update relevant documentation (e.g., README, API documentation, comments) for new features or changes. - **Screenshots/Videos (Optional)**: - Add screenshots or video recordings for UI changes (if applicable). - Ensure the UI matches the design and is responsive on different screen sizes. @@ -63,3 +74,4 @@ - **Self-Review**: The developer submitting the PR must perform a thorough self-review before assigning it to a reviewer. - **Cross-Environment Testing (Optional)**: Confirm the changes work in multiple environments (e.g., dev, staging, production). - **Feature Flags (Optional)**: Ensure new features are behind feature flags if they are not fully ready for release. + From 5aa22dec014f303255dc8c5a4adcd7cd483b5a57 Mon Sep 17 00:00:00 2001 From: Jatin Gupta Date: Thu, 23 Jan 2025 09:37:25 +0530 Subject: [PATCH 4/6] Remove optional mark for dependencies --- tech-standards/standard-PR-review-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tech-standards/standard-PR-review-checklist.md b/tech-standards/standard-PR-review-checklist.md index 1ded1be..7c9d82e 100644 --- a/tech-standards/standard-PR-review-checklist.md +++ b/tech-standards/standard-PR-review-checklist.md @@ -65,7 +65,7 @@ - **Dead Code**: Remove any unused code, variables, or imports. - **Debugging Artifacts**: Ensure no debugging code (e.g., `console.log`, `print`) is left in the codebase. -- **Dependencies (Optional)**: +- **Dependencies**: - Verify no unnecessary or outdated dependencies are introduced. - Lock file changes (e.g., `package-lock.json`, `requirements.txt`) should be reviewed. From dad5b7d9b683501d3f8aa17a703be30c50dd9bb6 Mon Sep 17 00:00:00 2001 From: Jatin Gupta Date: Thu, 23 Jan 2025 09:41:13 +0530 Subject: [PATCH 5/6] Add logging details --- tech-standards/standard-PR-review-checklist.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tech-standards/standard-PR-review-checklist.md b/tech-standards/standard-PR-review-checklist.md index 7c9d82e..e8c7699 100644 --- a/tech-standards/standard-PR-review-checklist.md +++ b/tech-standards/standard-PR-review-checklist.md @@ -38,6 +38,11 @@ - **Tests**: - Ensure unit tests or integration tests are added/updated for the feature or bugfix. - Run all tests and confirm they pass locally. + - Include both positive and negative test cases. +- **Logging**: + - Follow project's logging standards. + - Include appropriate log levels (DEBUG, INFO, ERROR). + - Avoid sensitive information in logs. - **Linting and Static Analysis**: - Code should pass all linting checks (e.g., ESLint, Flake8, etc.). - Resolve any warnings or errors flagged by static analysis tools. From eb8a62092acc53a684102d2c61828d6512911a77 Mon Sep 17 00:00:00 2001 From: Jatin Gupta Date: Thu, 23 Jan 2025 09:44:29 +0530 Subject: [PATCH 6/6] Update the dependices section --- tech-standards/standard-PR-review-checklist.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tech-standards/standard-PR-review-checklist.md b/tech-standards/standard-PR-review-checklist.md index e8c7699..ccb215c 100644 --- a/tech-standards/standard-PR-review-checklist.md +++ b/tech-standards/standard-PR-review-checklist.md @@ -73,6 +73,8 @@ - **Dependencies**: - Verify no unnecessary or outdated dependencies are introduced. - Lock file changes (e.g., `package-lock.json`, `requirements.txt`) should be reviewed. + - Check for security vulnerabilities in new dependencies. + - Ensure compatibility with existing dependencies. ## Final Checks