Peer Review Process
A comprehensive walkthrough of the peer review process as both best-practice philosophy and practical workflow. This session was recorded specifically for inclusion in the Greenfield curriculum to help new joiners develop the same high standards of code review that characterize the team. The format is deliberately conversational, with live questions and discussion throughout.
TL;DR
Peer reviewer is a rotating role — developers cycle through it for 2–3 weeks at a time. It’s a vital role, not a box-check gate; your review is "every bit as important as the acceptance review that’s going to happen after yours."
Core loop: check doneness (acceptance criteria met, builds in a shared environment) → offer professional, objective, concrete feedback → catch the details (changelog, version bumps, library/dependency bumps, docs/javadoc) → use it to learn the service.
Build and check out locally, not just in Bitbucket — Jenkins green isn’t enough. Local builds surface missing .gitignore entries, un-committed auto-generated template files, etc. Feedback should be objective and concrete, not subjective/style. Don’t review code style—focus on "does the code work as it should," not "would I have written it this way."
Tone is blameless and collaborative: phrase feedback as questions ("why did you choose to do it this way") or as "we should do it this way," not "you did this wrong." Reviews are a conversation, not a stamp.
The Rotation and the Mindset
Peer reviewer is a rotating role, picked up for a two or three week stretch. Most developers will cycle through it multiple times.
The common trap going in: "I just want to deliver code." The reframe: "You’re improving the overall quality of our delivered software." It’s a vital role, not a checkbox exercise.
It’s also a chance to increase your knowledge of a service — both business and technical. Early in a sprint when you have bandwidth is a particularly good time to dig in.
Focus Points: The High-Level Loop
When a PR comes in and you’re assigned it:
-
Check for doneness — Are the acceptance criteria met? Does it build in a shared environment? Can the delivering developer prove it’s at least done relative to the Jira ticket?
-
Offer professional, objective feedback to help the code along.
-
Consider it from various roles/perspectives — developer perspective, SRE perspective, etc.
-
Catch the details — changelog entries, version updates, documentation. Not the main thing, but critical for a peer reviewer to focus on.
-
Increase your knowledge of the service, business and technical.
Acceptance Criteria
Just the AC spelled out by the PO in the Jira ticket. Find them in the story and get your bearings on what the work was supposed to accomplish.
Study the code. Scope varies — sometimes a whole bunch of changes, sometimes minimal (a Java update or similar). Take your time, don’t rush.
Connect the changes from the Jira item to the PR. A great way: trace up and down the stack — especially if a resource class changed, trace from where an API call comes in all the way down through the code.
Successful Build
Verify via tracing the branch/ticket in Jenkins, or look at the PR itself for the green check that the build passed.
Rule of thumb: confirm it’s building before you go too far down the road as reviewer.
Detailed Considerations: Digging Into the Code
Don’t be afraid to dig in. Get real thoughts in—this is a professional review, not a bare-minimum check.
Objective Over Subjective
Concrete suggestions beat interpretable opinions. "There’s five different ways to do most things." Feedback should be a real, concrete thing to act on.
When you have meaty feedback, offer up code — a snippet or two that explains your thinking when asking for changes.
Performance & Integration Considerations
Often overlooked by both devs and reviewers: are there new services being integrated with? Are you adding load to the system?
Make sure configuration correlates with the change: default config settings, max connections, timeouts, etc.
Consider the code from all perspectives — maybe you need to add/change/tweak a property (e.g., max connection settings) beyond just "these methods work a certain way."
The Details Pass
Once feedback has been given and the code is converging:
Versioning & POMs
Forgetting a client version is easy. Code may not break (you don’t pull it), but make sure every version you updated — and the version of the service being updated — is bumped wherever it’s referenced throughout the configuration. A reviewer catching this frees the PO to focus on delivered functionality.
Libraries & Dependencies
Keep an eye out for updates, especially second- or third-digit version bumps. Over the 2–3 weeks you’ll start to remember "oh, mobile framework just updated" or "Tiles just updated." Not hard-and-fast, but keep dependencies moving.
Use It as a Chance to Learn
Unfamiliar with the service? Dig into the documentation — business and technical. It helps you orient.
Encourage taking stories across services and specialties within the stack.
New tech or new SDK you’ve never seen? Take a minute to learn it.
Ask questions in Slack, build locally, run the service and integration tests locally to see what’s going on.
Local Build vs. Bitbucket-Only Review
Lighter changes (Java compatibility bumps, newest mobile framework, integration/config) can move quickly and be reviewed on Bitbucket directly.
Weighty changes → clone the service, check out the branch, review locally. It helps a lot on big changes.
Why Build Locally?
Even when Jenkins is green, a local build surfaces things a pass/fail can’t:
-
A file that needs to be added to .gitignore
-
An auto-generated template file that changed but wasn’t part of the PR — "we have so many automated things"
There’s the code changed in the PR, and there’s other stuff that might not have been changed. Build/check out locally to confirm nothing was missed. The dangerous failure mode: "nothing looks wrong" convinces you, but something that should have been included isn’t (and won’t show in the diff).
This is especially important for skaffold/kustomize changes where things get missed.
Things to Avoid as a Reviewer
-
Finding balance — You don’t need to go all-in and pick out every little thing. Sometimes code is "done a way you wouldn’t do it but not necessarily wrong." How far you go is calibrated by doing it a few times.
-
Just checking PR boxes — Your review is every bit as important as the acceptance review after yours. It’s not a gate.
-
Offering only opinion and style — Make feedback objective and concrete.
-
Being overly critical on fine details — You don’t need to give every bit of feedback just because you know them.
-
Assuming Bitbucket-only is enough — Favor local builds as much as practical.
Bitbucket Comment Mechanics
Start review (relatively recent feature): comments stay pending within the PR until you hit complete review — this saves a flood of notifications to the dev. For a big PR with lots of feedback, however, it may be better to comment serially so each notification goes out immediately and the dev can start addressing right away. This is a per-reviewer style call.
Comment vs. Task: a comment is a note to the dev (sends a notification). A task is something you want changed — a conversation/agreement that it needs to change. There’s a checkbox to add a comment to a pending batch vs. send immediately.
Workflow: Jira & Bitbucket Integration
-
Reviewer gets assigned a peer review subtask on the story; move it to In Progress.
-
Open the story, read description + AC to get a flavor of what to look for.
-
Find the PR via the Bitbucket link at the top of the story (development heading). A correctly tagged PR title auto-links back to Jira, so the link being there confirms the PR was set up right.
-
First check: verify a Jenkins build happened (green check) before going further.
-
Bounce through changed files on the left side of the diff. Look for resource classes or key changes to understand what’s coming throughout the PR.
-
Hit + on a line to add a comment; decide whether it’s a comment (immediate note) or a task (wants change). Add to pending batch or send immediately.
-
As you go, check: versions updated correctly, mobile framework updates, changelog entry present, docs updated.
-
Wrap up with a summary comment — "small changes requested, please… overall looks good" — it’s a conversation, not a one-way stamp.
-
Publish with a resolution:
-
No status = general "here are a few things to think about"
-
Need-work = things must change before approval
-
Approve = thumbs up, back to dev
-
-
Back on the Jira subtask: if approved, move it on to the PO; if waiting on back-and-forth, move to Waiting and pick up your next reviewer task.
Best Practices from the Floor
For Reviewers
Phrase feedback as questions rather than directives. "Why did you choose to do it this way?" opens dialogue better than "you did this wrong." Frame suggestions as "we should do it this way." Tone through PRs is super important; it’s a collaboration, not confrontation.
Focus on "does the code work as it should," not on code style. Your style may differ from the author’s; that’s not a PR issue.
For Developers Receiving Feedback
Don’t take feedback personally. Read comments "in the most positive way you could take a comment," not the worst. Same goals — deliver the best product.
Give each other grace. A developer may have struggled hard with a story; even when feedback isn’t personal it can feel personal after you’ve "beaten your head into a brick wall for a week."
For Developers Delivering
Add read-ahead comments to your own PR explaining your choices. "I chose to do this because of this… this was more performant." This hugely helps both the reviewer and the PO understand choices, especially since "there’s five different ways to do everything" and Bitbucket doesn’t always render the diff well. It also helps discern "tech debt while you’re in there" changes from story changes.
Accounting for Review Time
Account for the review process in your story points and timelines. When work needs to go to multiple teams for multiple peer review approvals, that takes time and people often don’t account for it in time-to-close.
Handling Oversized PRs
Nightmare PRs usually trace back to an over-generalized ticket that should’ve been broken down. When you hit one: take it back to the PO — flag that the PR was huge so they make stories smaller/more bite-sized going forward. If a huge story isn’t finished and needs splitting, loop in the PM + PO to break it into smaller pieces.
Also: be kind — give a heads-up ("block out some time") before dropping a big PR on someone.
If a PR is huge, it’s often a signal about the ticket itself: stories should be smaller and more focused.
Acceptance Criteria Clarity
Handle AC clarity during planning — POs bring stories, devs ask as many AC questions as possible when picking them up. You won’t understand AC perfectly on first read; it’s an ongoing back-and-forth. Planning is the place to ask.
When reviewing a ticket’s AC, check the service for "you’re going to shoot yourself in the foot because you missed this" gotchas and call those out explicitly.
Why This is in the Knowledge Base
This is the canonical statement of the peer review expectations and the reviewer rotation — the doneness loop, the local-build-don’t-trust-Jenkins rule, objective-not-style feedback, blameless tone, and the Bitbucket/Jira mechanics. It’s explicitly destined for the Greenfield curriculum to help new joiners develop the same high standards, and it fills a critical knowledge gap about "how reviews actually work here," not just the mechanics but the mindset. Useful any time someone joins the rotation, a debate flares over how deep a review should go, or a new hire needs to understand the culture of code review.