nigel_peters@lemmy.nztoProgramming@beehaw.org•What does your merge/code review process look like?
1·
1 year agoOurs is pretty intense - large bank, 60 or so iOS engineers actively contributing to a mono-repo:
- We have about 15 CI steps that pick up on anything from basic linting to security concerns (SonarQube). Unit tests, UI tests, etc.
- We have a template that PR authors follow to add descriptions, test plans, devices tested on.
- Reviewers are automatically assigned using a round robin system
- Reviewers obviously review the code, but also execute the test plan, which includes accessibility testing.
- All PRs require 2 approvals.
- A bunch of other stuff (uploading artefacts, generating gRPC protos) that probably isn’t worth going into detail.
It’s intense, and PRs on average take a week or so to get merged. In saying that, it is the highest quality and most well-architected codebase I have ever worked on.
If I were in your situation I’d push for the following:
- all PRs have one approval, preferably two depending on team size
- code is tested by someone else before being merged to main
- use linters, Danger, etc to pick up on trivial shit
- a few manual checks like ensuring code is unit tested
- a Github PR Reviewer guide describing common issues to look for, tone of messaging when leaving comments (“be nice”, “make it clear when you are adding optional nit-picks”, etc)
- encourage authors to add review comments to their own PRs for any bit of code that isn’t immediately obvious
- stretch goal: look into generating code coverage reports on your PRs, add quality gates
I’m pretty sure I’ll be getting one, but then again I’m an iOS developer and will likely be playing around with developing apps for it.
I can definitely see myself using this in the (home) office and around the house. Purely for productivity this looks like it’ll be amazing