From f1dff6f8cdc84298adbae7708963f4cd618a04f3 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 7 Aug 2026 07:19:02 -0700 Subject: [PATCH 1/2] Update PR template with code review checklist --- .github/pull_request_template.md | 47 +++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d9a3efde48..d54cdc31ad 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -2,13 +2,6 @@ - - -- [ ] Tests for the changes have been added -- [ ] Related documentation has been added / updated -- [ ] OSS packages added to Curious [open source credit page](https://mindlogger.atlassian.net/wiki/spaces/MINDLOGGER1/pages/263127186/Admin+Panel+Applet+Builder+Library) -- [ ] Delivered the `fix` or `feature` branches into `develop` or `release` branches via `Squash and Merge` (to keep clean history) - ### 📝 Description @@ -59,3 +52,43 @@ Replace this line with anything else you think may be relevant or related PRs If there are no notes, then delete this section. --> + +### ✅ Checklist + +### Functionality + +- [ ] The feature behaves correctly in practice and fulfills the intended business purpose +- [ ] The implementation accounts for edge cases, avoids subtle logical errors, and handles somewhat rare failure states (e.g. offline mode for mobile, 3rd party being down, etc) + +### Testing + +- [ ] Verify there are automated tests added that meaningfully cover critical behavior and failure cases +- [ ] Code coverage does not go down as result of this change +- [ ] Test suite passes + +### Security & Data Privacy + +- [ ] Verify there is no chance we would accidentally log PII to application logs +- [ ] Verify this addition does not materially affect our security attack surface, and if so it has undergone security review +- [ ] All inputs are sanitized +- [ ] New dependencies are well maintained, have significant justification for being added to the project, and are documented in the Curious [open source credit page](https://mindlogger.atlassian.net/jira/servicedesk/projects/MLA/knowledge/articles/340623543?spaceKey=MLA) + +### Logging/Monitoring + +- [ ] Logging is implemented for this change such that you could troubleshoot this feature in production +- [ ] The change/feature is able to be monitored in production + +### Performance + +- [ ] This change does not introduce n+1 queries or other performance issues within our expected scale (e.g. missing indexes on frequently queried columns, frequently updating tables that are accessed often) + +### Readability + +- [ ] All commented out code is removed +- [ ] Debugging code including extraneous log lines are removed +- [ ] Code is easy to understand through naming and structure; comments explain intent or non‑obvious decisions + +### Change Safety + +- [ ] Backend changes are backwards compatible with old clients, or it is well known they are not and a deployment/rollout plan is in place. This include backend changes being compatible with old mobile app versions, as well as applet versioning within Curious. +- [ ] Destructive database migrations are rolled out in stages. For example, renaming a column means adding a new column and migrating the existing data to that columns in one deployment. Then monitoring to ensure that field isn’t used, and finally removing that old column in a separate deployment. From 40ca85bf03f412e12193cb9c2e6d8b1ac63601f8 Mon Sep 17 00:00:00 2001 From: Chris Lei Date: Fri, 7 Aug 2026 07:23:57 -0700 Subject: [PATCH 2/2] Cross out n/a items in PR code review checklist --- .github/pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d54cdc31ad..4fab5fb4f9 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -90,5 +90,5 @@ If there are no notes, then delete this section. ### Change Safety -- [ ] Backend changes are backwards compatible with old clients, or it is well known they are not and a deployment/rollout plan is in place. This include backend changes being compatible with old mobile app versions, as well as applet versioning within Curious. -- [ ] Destructive database migrations are rolled out in stages. For example, renaming a column means adding a new column and migrating the existing data to that columns in one deployment. Then monitoring to ensure that field isn’t used, and finally removing that old column in a separate deployment. +- [x] ~~Backend changes are backwards compatible with old clients, or it is well known they are not and a deployment/rollout plan is in place. This include backend changes being compatible with old mobile app versions, as well as applet versioning within Curious.~~ +- [x] ~~Destructive database migrations are rolled out in stages. For example, renaming a column means adding a new column and migrating the existing data to that columns in one deployment. Then monitoring to ensure that field isn’t used, and finally removing that old column in a separate deployment.~~