Simple checklist for reviewers and quality leads #1
Conversation
| ### 📊 Monitoring & Metrics | ||
| - **How will you monitor this release in production?** | ||
| _List dashboards, alerts, or logs you’ll watch._ | ||
| ### Gatekeepers (must all be ✅ before merge) |
There was a problem hiding this comment.
Is this for gatekeepers? Quality leads that must approve the code?
There was a problem hiding this comment.
Yes, so they know that the pr is ready for the final review and deployment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I agree, that generally it is true. But in case they need to approve it, this may be helpful, wdyt?
| ### AI-Generated Code | ||
| - [ ] This PR includes AI-generated code | ||
| - [ ] **I verified correctness of the implementation** | ||
| - [ ] **I verified that code meets Blinkist standards** |
There was a problem hiding this comment.
This might be a bit vague perhaps. And I think we should always assume there's AI generated code at this point.
There was a problem hiding this comment.
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.
| - [ ] I have performed a self-review of my own code | ||
| - [ ] SLO or monitor exists that would detect a failure |
There was a problem hiding this comment.
❓ 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
Here is a simple checklist to simplify review by teammates and quality leads.
The goal is to make it simple yet helpful for providing information about context and safety of the release.
Why
The checklist makes explicit that we require monitors, use AI responsibly and think about rolling out, so engineers can self-certify before merging and reviewers can focus on high-risk areas.