Stefano Magni
I'm a passionate Front-end Engineer, a speaker, and an instructor. I love creating high-quality products, testing and automating everything, learning and sharing my knowledge, helping people. I work remotely for Hasura.
A Pull Request Journey
PR Review WG
Stefano Magni - Rob Dominguez - Daniel Chambers
The problem:
Our PRs
stay open
for 7 days
Stats
10 interviews
6 meetings
3 documents
Slides are boring!!
Storytelling, please!!!!
The story is fiction 😊
But the problems
are real 😊
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
💡
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
💡
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
💡
1 minute later...
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
I know how
to help you...
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
I know how
to help you...
CODE OWNERS!
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
1 day later...
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
1 day later...
2 days later...
❓
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
1 day later...
2 days later...
3 days later...
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
1 day later...
2 days later...
3 days later...
💡
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
1 day later...
2 days later...
3 days later...
💡
New task!
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
CODE OWNERS!
1 day later...
2 days later...
3 days later...
New task!
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
1 day later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
1 day later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
1 day later...
1 day later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
1 day later...
1 day later...
1 day later...
👨💻
👨💻
1 minute later...
2 days later...
2 days later...
1 day later...
1 day later...
1 day later...
1 day later...
👨💻
1 minute later...
2 days later...
2 days later...
1 day later...
1 day later...
1 day later...
1 day later...
👨💻
1 minute later...
1 week later...
👨💻
1 minute later...
1 week later...
1 month later...
1 minute later...
Let's recap the problems!
1 minute later...
Let's recap the problems!
🚨 No PR description 🚨
1 minute later...
Let's recap the problems!
🚨 No PR description 🚨
🚨 Big PR 🚨
1 minute later...
Let's recap the problems!
🚨 No PR description 🚨
🚨 Big PR 🚨
🚨 No direct reviewers 🚨
🚨 Notification missing 🚨
🚨 Reviewers have no time 🚨
🚨 Timezone incompatibility 🚨
🚨 CI issues 🚨
How
to fix them?
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
💡
1 minute later...
🚨 No PR description 🚨
✅ Describe your PR in details ✅
✅ Write tests ✅
🚨 No PR description 🚨
👨💻
👨💻
2 days later...
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
CODE OWNERS!
🚨 Notification missing 🚨
✅ Assign the PR to individuals ✅
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
🚨 Notification missing 🚨
✅ GH Slack integration + Haystack report ✅
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
🚨 Big PR 🚨
✅ Reduce PR size ✅
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
👩💻
👨💻
1 minute later...
🚨 Reviewers have no time 🚨
✅ Explicitly allocate time for reviews ✅
👨💻
👨💻
2 days later...
🚨 Timezone incompatibility 🚨
✅ Assign PRs to same-team peers ✅
👨💻
1 day later...
1 day later...
1 day later...
1 day later...
🚨 CI issues 🚨
🎉 There's a dedicated WG 🎉
PR authors must
- Reduce the scope of the PRs
- Describe PRs in details
- Write tests
- Assign PRs to individuals from their own team
- Do not start working on new tasks until the PR is merged
- Use Github's Slack integration
PR reviewers must
- Dedicate some time slots every day to review other people's PRs (the cross-team ones too)
- Report the tested functionalities
- Use Github's Slack integration
Team managers must
- Understand that PR reviews are a team's responsibility, not just an individual's responsibility
- Retrospectively understand if/why people need a lot of time to review PRs
- Enable Slack reports using Haystack for the team PRs
Guess what?
A document with these best practices already exists!
What happens if we do not apply
these best practices?
That we start working on a lot of things
but we release just a few...
A lot of bugs hit our users
That a lot of PRs remain orphans and
we need to automatically close them
What about the ideal situation?
More pair programming
No blocking reviews but post-mortem reviews
A lot of trusted and stable tests
Fast and reliable CI pipelines
PRs are released in production right after being merged
What's next?
✅ Include PR reviews
in the onboarding docs
⏳ Check how long the PRs
stay open on a monthly basis
⏳ Spread the word and the resources
WE NEED YOU!
PR reviews cannot be improved
without the individual efforts
of each PR author and reviewer!
Thank you!
PR Review WG
Stefano Magni - Rob Dominguez - Daniel Chambers
For getting interviewed
Nikunj - Abby - Tom Harding - Nithin - Priya
Jesse Hallett - Solomon - Shraddha
Daniele - Karthikeyan
For participating
Aaron Johnson - Shraddha Agrawal - Auke - Bilal - Brandon Martin - Brandon Simmons - Daniel Chambers - Daniel Harvey - Greg - Aravind KP - Manas - Marion - Matthew - Nicolas Beaussart Hatchuel - Ojas - Rikin - Samir - Solomon - Stef - Vamshi - Vijay
PR authors
- Reduce the scope of the PRs
- Describe PRs in details
- Write tests
- Assign PRs to individuals
- Do not start working on new tasks
PR reviewers
- Dedicate some time slots
- Report the tested functionalities
- Use Github's Slack integration
Team managers
- Get the PR to review a team's matter
- Enable Haystack reports for PRs
Hasura PR Review WG presentation
By Stefano Magni
