+
Skip to content

Simple checklist for reviewers and quality leads #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+19 −40
Open

Simple checklist for reviewers and quality leads #1

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9710cd8
Simple checklist
llxff Sep 5, 2025
a9f6306
Update pull request template for clarity and standards
llxff Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 19 additions & 40 deletions pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,30 @@
## 🚦 Release Freeze Checklist
**[Jira Ticket](https://blinkist.atlassian.net/browse/CK-XXX)**

This PR is being created during a **release freeze**. Please answer the following questions clearly before requesting review:
**Change Type:** <!-- Feature / Bugfix / Refactor / Docs / Etc -->

### 🔍 Risk Awareness
- **What does this PR change, and why is it critical enough to go out during a freeze?**
_Provide clear justification._
**Domain:** <!-- Payments / Auth / Subscriptions / Content / Etc -->

- **What is the risk level of this change (low / medium / high)? Why?**
_Be explicit about possible failure modes._
## Description

---
<!-- What is changing and why? -->

### 📊 Monitoring & Metrics
- **How will you monitor this release in production?**
_List dashboards, alerts, or logs you’ll watch._
### Gatekeepers (must all be ✅ before merge)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for gatekeepers? Quality leads that must approve the code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, so they know that the pr is ready for the final review and deployment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, this is going too far. "Quality leads" can't and shouldn't review all PRs. It's about being aware of what we are shipping, trusting each other's expertise, so a final PR review wouldn't be needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that generally it is true. But in case they need to approve it, this may be helpful, wdyt?


- **What metrics or monitors will help detect if something goes wrong?**
_E.g., error rates, latency, conversion drops._
- [ ] I have performed a self-review of my own code
- [ ] SLO or monitor exists that would detect a failure
Comment on lines +13 to +14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ This assumes that for every change we'll be able to have an SLO or monitor, which won't always be the case.

I'd suggest to add an alternative to indicate there is no need for a monitor


- **Do you have an alert configured that would notify the right people if an issue arises?**
_Yes/No, details if yes._
### AI-Generated Code
- [ ] This PR includes AI-generated code
- [ ] **I verified correctness of the implementation**
- [ ] **I verified that code meets Blinkist standards**
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a bit vague perhaps. And I think we should always assume there's AI generated code at this point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should always assume it. This is to make sure the author explicitly confirms that they reviewed correctness of the code and that it follows our standards.

So it is about making sure accountability stays with the author and isn't blurred with AI.


---
### How has this been tested?

### 🧪 Testing & Rollout
- **How has this change been tested (unit, integration, staging, manual)?**
_Add links to test results if possible._
- [ ] Tested on staging
- [ ] Tested locally

- **Is there a way to safely roll this out (feature flag, canary, % rollout)?**
_Describe rollout plan._
### Observability

- **What is the rollback plan if something goes wrong?**
_Be specific about steps and impact._

---

### 👥 Ownership & Communication
- **Who is responsible for monitoring this change after release?**
_List names/teams and handover details._

- **Have the relevant stakeholders been informed (QA, Engineering, Product)?**
_Yes/No, mention names if yes._

---

✅ **Please confirm:**
- [ ] I have reviewed the risks and monitoring plan with my team.
- [ ] I am confident this change is safe to release during the freeze.

---
- Dashboards updated/created: <!-- links -->
- SLOs updated/created: <!-- links -->
- Key metrics to watch post-deploy: <!-- list -->
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载