Giving and Receiving Feedback Questions

Direct answer

I'd talk to the teammate privately, describe the specific pattern and its effect on the team, ask what is making reviews slow, agree a small concrete change, and track a number before and after. A pull request (PR) is a proposed code change that teammates review before it is merged, so a slow reviewer holds up everyone's work.

Prepare

• Pull the data: how long PRs wait for their first review. I'd call that the time to first review.
• Check causes: are they buried in their own work, getting huge PRs, or in a different time zone? The fix depends on the cause.

Deliver

"Over the last month, PRs assigned to you waited a median of about two days for a first look. Two teammates told me their work was blocked while waiting. I'd like us to get that down. What's getting in the way, and what would help?"

Possible agreements: a fixed 30-minute review slot each morning, a one-working-day target for a first response (even just "I'll look Thursday"), or asking authors to keep PRs small.

How I'd tell whether it helped

Measure the same thing before and after. Illustrative numbers (hours to first review, five PRs each):

• Before: 52, 70, 30, 66, 48. Median 52 hours.
• After: 20, 6, 26, 4, 10. Median 10 hours.

The median drops from 52 to 10 hours. I'd use the median since one very slow PR distorts an average. With only five PRs, it is evidence, not proof, so I'd look again over a couple of weeks.

I'd also check that quality did not drop: are reviews still catching problems, and are teammates reporting fewer blocked days?

Pitfalls

• Public shaming or a leaderboard of reviewers.
• Making speed the only goal, so reviews become rubber stamps.
• Skipping the conversation and quietly reassigning their reviews.

Direct answer

I lead with the risky bug, label it clearly as blocking, explain why it matters and suggest a fix, and then mark everything else as non-blocking (nits and optional ideas) so the author can see what must change before merge. I keep the minor style issues short, grouped, or handled by an automated formatter, so they do not bury the one comment that matters.

Sequence and structure

1. Top summary comment: "One blocking item: possible race condition in save_order (a bug where two requests running at the same time interfere with each other). Everything else is non-blocking: nits and optional suggestions that will not hold the merge."
2. The bug comment, first and prominent: say what could happen, when, and a possible fix. "Blocking: two requests can both pass the stock check and both decrement, leaving negative stock. Could we do the check and update in one transaction (a group of database steps that succeed or fail together)? Happy to pair."
3. Label the rest. Google's engineering practices recommend prefixing comments with a severity label: "Nit:" for a minor point (their wording: "Technically you should do it, but it won't hugely impact things"), "Optional" or "Consider" for an idea that is "not strictly required", and "FYI" for something to think about later. Note the difference: a Nit is small and worth fixing at some point (at the latest the next time the author touches that code) but never holds up the merge, while Optional is genuinely skippable. On my team I would say in the top summary which labels I will not hold the merge for, so the author is never guessing.
4. Reduce the volume. Instead of ten comments on the same naming habit, leave one: "Nit: several variable names use abbreviations; same throughout, optional to fix." Better still, ask for a linter or formatter (an automatic tool that enforces style) so humans stop discussing spacing.

Worked example

Out of 12 observations, 1 is the race, 8 are naming and spacing, 3 are small refactors. What the author sees: 1 comment labeled Blocking, 1 grouped Nit: comment covering the 8 style points, and 3 comments labeled Optional. In the review tool, "Request changes" is a status that blocks the merge until the author responds, and "Approve" lets it go ahead. The status is "Request changes" only because of the one blocking item; if the author fixes it, I approve even if the nits remain.

Trade-offs and pitfalls

• A pile of equal-weight comments hides the one important one.
• Marking everything blocking trains authors to argue about everything.
• If the nits are really about unclear code, they might be maintainability concerns; promote those to "Consider" with a reason rather than "Nit".

Direct answer

I would raise it privately and early with the developer, using a short three-part note (situation, behavior, impact), bring the cost data, offer a concrete way to fix the tests, and then agree on a date when we both look at the numbers again. If nothing changes, I involve their manager as a shared-problem conversation with the same data, and I fix my own side so their flakiness cannot wake my on-call at night regardless. Feedback only "changes" when the person knows the cost, knows exactly what to do, and sees a follow-up.

How I would do it

Terms: a flaky test passes and fails on the same code with no change, usually from timing, shared state (data that several tests read and change) or network calls. A spurious alert is a page that fires with no real customer problem. On-call is the rotation of engineers who get paged out of hours. The pipeline is the automated build-and-test run that checks every code change. Main-branch runs are runs of that pipeline on the team's shared, up-to-date code, so a failure there is not caused by one person's work in progress. A retry automatically re-runs a failed test once before reporting it.

1. Check my own picture first. Is the test really theirs? Is it flaky or is it failing for a real, intermittent bug? Do they even know it pages anyone? Often the author never sees the alert, so the cost is invisible to them.
2. Collect evidence they cannot argue with. Illustrative numbers: the test failed on 9 of the last 40 runs on the main branch (22.5%), and it triggered 3 pages in two weeks, on 3 separate nights, each costing about an hour of someone's sleep.
3. Talk one-to-one, before escalating. Peers outside my reporting line owe me nothing, so tone is "help me understand" rather than "fix it". Use SBI (Situation, Behavior, Impact, a model from the Center for Creative Leadership).
4. Make the fix cheap. Offer a pattern or offer to pair for 30 minutes. The usual three: (a) replace a fixed sleep with waiting until a condition is true, (b) give each test its own data so tests do not disturb each other (isolate shared state), (c) replace a real network call with a fake that returns a fixed answer (mock it). Agree a stopgap: quarantine the test (it still runs, but its failures no longer page anyone) until fixed.

# Flaky: guesses the job finishes within 2 seconds
time.sleep(2)
assert order.status == "done"

# Stable: checks repeatedly, up to 10 seconds, and continues as soon as it is true
wait_until(lambda: order.status == "done", timeout=10)

wait_until is not a built-in: it stands for a small helper your test suite defines (or a retry library provides) that re-checks a condition every fraction of a second and fails only if the timeout passes. The snippet is a pattern sketch, not a runnable program.
5. Close the loop. Agree a check-in date, look at the flake rate together, and thank them publicly when it drops.

Worked example

"Yesterday's 02:10 page came from test_checkout_timeout in your service's pipeline (Situation). It failed on 9 of the last 40 main-branch runs with no code change. The pipeline's single automatic retry hides most of those failures, but when the retry fails too the alert fires, and it did (Behavior). That has cost our on-call three broken nights in two weeks (3 pages on 3 separate nights), and the team is starting to ignore that alert channel (Impact). Could we look at it together on Thursday? I think it is waiting on a fixed sleep, and I can show the condition-wait pattern we use."

All three SBI parts are present, there is an ask, and no judgement of the person. The numbers tie together: 9 failures in 40 runs is 22.5%. If each run failed independently at that rate, about 5% of runs (0.225 x 0.225) would fail twice in a row and page, roughly 2 pages per 40 runs, which is the same order as the 3 pages observed (illustrative figures).

Written work with a repeated downstream cost

The same method fits a colleague's incomplete model documentation. Situation: the churn-model README for the latest release. Behavior: the input section does not state that tenure is in months or how nulls are encoded. Impact: my team guessed days and shipped the same integration bug twice. The ask is a short checklist (units, null handling, ranges, an example row) plus an offer to review the next draft.

Trade-offs and pitfalls

• Escalating first poisons the relationship; waiting forever lets the cost grow. One direct conversation and one dated follow-up is the balance.
• If they say "it is low priority", the answer is data plus their manager, not more nagging.
• Fix my side as well (alert only on non-quarantined tests): it is faster than waiting on another team's behavior.
• If the failure turns out to be a real intermittent bug, the conversation becomes "thank you for the signal" and the tone changes.

Direct answer

I would separate the system conversation from the personal one. Hold a blameless review (one that asks how the system allowed the failure, not who to blame) for the incident, then have a private one-to-one with the teammate that describes what happened and its impact, asks for their account, and agrees on specific changes. Afterwards I fix what made the shortcut attractive and follow up so the feedback is not a one-off.

Terms used here

• Schema migration: a change to the structure of the database (adding or changing tables and columns) that runs against live data.
• Dry run: a rehearsal of the migration against a copy or in a no-changes mode, to see what it would do before it touches real data.
• Guardrails: checks built into the process (required reviews, pipeline gates) that make the unsafe step hard to take by accident.
• Postmortem: a written report after an incident covering what happened, why, and what changes.

Before the conversation

1. Let the incident finish and the adrenaline settle, but do not wait weeks.
2. Gather facts: what the shortcut was, what pressure led to it, and which guardrails were missing.
3. Check my own part: was the deadline mine, was the check optional, did anyone approve it?

The conversation (SBI, Situation, Behavior, Impact)

"Thanks for making time. Situation: last Thursday, during the release. Behavior: the schema migration ran without the dry run. Impact: that took the order service down for 40 minutes. I would like to hear what you were weighing at the time."

Then listen. Often the shortcut had a reason, such as a slow staging environment or a deadline. Ask: "What would have made the safe path the easy one?" End with a joint commitment, for example "we add the dry run to the pipeline so it cannot be skipped."

Keeping it from feeling like blame

• Focus on actions and conditions, not character ("skipped the dry run" not "careless").
• Say what I would have done in their position, if true.
• Share the postmortem finding publicly in system terms. The private feedback is separate, and I do not name them in the team write-up unless they agree.
• Distinguish an honest shortcut under pressure from a repeated disregard for known rules. The latter is a performance issue, handled separately and more firmly: "This is the second time the dry run was skipped after we agreed it was required. I need it followed every time. What is getting in the way?"

What I do afterwards

1. Make the safe path the default (pipeline check, required review) so the fix does not rely on willpower.
2. Track the action items to completion with owners and dates.
3. Check in one week later: "How did the new check feel? Anything slowing you down?"
4. Notice and credit the next time they raise a risk early.

Result I look for

They raise the next risky shortcut before they take it. That signal shows that psychological safety (feeling safe to admit risks and mistakes) has survived the incident.

Direct answer

I would give the feedback privately, in person first and then in a short written note, using specific examples of what stakeholders could not find. I would ask for one concrete change (a plain-language summary at the top), and I would check it landed by seeing whether a stakeholder can use the next postmortem without asking an engineer.

A postmortem is a written report after an incident: what happened, why, and what changes. Stakeholders are the non-engineers who rely on it, such as support, product or management. The jargon in the note below: a database failover is the automatic switch to a backup database when the main one fails; replication lag is how far the backup was behind the main database, shown in graphs; a config diff is a line-by-line comparison of settings before and after the change. All three matter to engineers and tell a support lead little about customers.

The conversation

1. Open with what is strong: the analysis is accurate.
2. Describe the observable gap: where the impact sits, and a real instance of a stakeholder asking for help.
3. Ask what audience they had in mind. A senior engineer may have written it for other engineers on purpose.
4. Agree on the change and on a date.

The written note (SBI: Situation, Behavior, Impact)

Hi Maya, I read the database failover postmortem from last week (situation). The technical analysis is sound, and the timeline is the clearest I have seen. The first page opens with the replication-lag graphs and the config diff, and the customer impact appears in section 4 (behavior). Our support lead told me she could not tell from it how many customers were affected or whether it could happen again, so she asked me instead of reading it (impact). Could the top of each postmortem have four lines: what happened, who was affected and for how long, whether it is fixed, and what we are changing? The detail can follow. Would you try it on the next one? I can read the draft first.

The note contains the three parts (situation: the failover postmortem; behavior: impact buried in section 4; impact: the support lead had to ask) plus a specific request and an offer to help. It is under 150 words and avoids words like "unreadable."

What a stakeholder-usable postmortem has at the top

The engineering detail stays below, as it is.

How I tell whether it landed

• Direct signal: I ask a stakeholder (the support lead) to read only the top of the next postmortem and tell me the impact. If she can, it worked.
• Indirect signal: fewer "what does this mean for us" questions arrive in chat after a postmortem is published.
• Behavior signal: the next postmortem opens with the summary without a reminder.
• If it did not land: I ask what got in the way. It may be time, uncertainty about how much to simplify, or disagreement, and I respond to that cause.

Pitfall

Do not rewrite their postmortem for them, since it fixes the document without fixing the habit.