Giving and Receiving Feedback Questions

Direct answer

I would ask a short question that shows I want to get it right, not that I disagree: what outcome the reviewer is after, whether it blocks the merge, and what an acceptable change would look like. If the thread has gone back and forth twice, I would take it to a short call or a desk chat and write the conclusion back in the thread.

Example

Comment on a PR (pull request, a proposed code change awaiting review): "This feels fragile." It sits on a function that parses a date.

Clarifying reply in the thread

Thanks for looking at this. I want to make sure I fix the right thing. Is the concern that the parser breaks on dates in other formats, or something about how the function is structured? Is this blocking the merge, or could it be a follow-up? If you have a rough sketch of what you would prefer, I am happy to start from that.

Why the wording works

• It starts with thanks and states the goal ("fix the right thing"), so it reads as engagement, not defensiveness.
• It offers two concrete guesses, so the reviewer can answer yes or no.
• It asks about blocking status, which is the most useful single fact.
• It asks for an example, since "show me" is quicker than explaining.

Thread or in person

• Thread: when the question is narrow and the answer will help later readers.
• In person or call: when the first reply is still unclear, the reviewer seems frustrated, or the change is large. Afterwards, post a summary: "We agreed to handle the other date formats in this PR and leave the refactor for a ticket."

Avoid

"What do you mean?" with no context, or "This is how the other code does it," which reads as defending. Do not guess silently and push a change, since you may fix the wrong thing and add another review round.

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 walk through a real review where I opened with the specific problem, worded it as a question with a reason and an offer of help, then followed up in person and checked that the fix landed. The PR (pull request) was fixed and the relationship stayed intact. Here is how I would tell it. The specifics are illustrative; in an interview use a real review of your own, or say plainly that you are describing how you would handle it.

Situation

A teammate about four months into the role, who had gone quiet in meetings after an earlier review thread got heated, submitted a change to a deployment configuration (a file defining how the service is rolled out). It set the memory limit (the most memory the service may use before the platform kills and restarts it) so low that the service would be restarted once traffic reached a busy-day level, and removed a readiness check (a probe that tells the platform whether an instance has started and can take traffic; a separate liveness check tells it whether a running instance is stuck). That was a real production risk, and I was the reviewer.

Actions I took

1. Decided what mattered. Two blocking points (memory limit, health check) and a few optional ones. I only wrote the blocking points in the first pass.
2. Worded it carefully. For example: "Blocking: with a 128Mi limit (128 mebibytes, about 134 MB of memory), I think the service will be killed once traffic rises, because the last load test, which simulated a busy day, showed it using about 300Mi. Could we raise it, or check the numbers together? Happy to pair." For the health check: "Could we keep the readiness check? It tells the platform an instance is ready to receive traffic. Without it the platform sends traffic to instances that haven't started yet."
3. Added real praise. "The rollout order is cleaner than the old file; thank you for splitting it up."
4. Talked in person after posting, in 10 minutes: "I left a couple of blocking comments. They are about the config, not about you. Want to go through them?"
5. Offered support. A pairing slot, plus a link to the team's config checklist.
6. Made sure it was acted on. I checked the PR the next morning; the new commit raised the limit and restored the check, and I approved.

Result

The PR merged within a day. Afterwards they messaged me: "Thanks, I would never have spotted the load test number. Can you look at my next draft before I open the PR?" They started sending drafts early, which told me the relationship was healthy.

If the issues had repeated

I would say so directly in a private one-on-one: "This is the second time the health check was dropped. What's making it easy to miss?" Then add it to the checklist or a pre-merge check. If it kept recurring after that, I would tell their manager what I had observed and what support I had offered, as a shared problem, not as a complaint.

What I would do differently

I would have talked to them before posting the comments, since I knew they were sensitive to written criticism.

Trade-offs and pitfalls

• Too gentle: the bug ships. Too blunt: the person stops sharing work early.
• Doing the fix myself would have been faster but they would not have learned.
• Praise must be true or it costs trust.

Direct answer

I'll tell this as a story (illustrative skeleton). A peer on my team kept merging changes (adding code to the shared codebase so it goes live) late on Friday afternoons without telling the on-call engineer (the person on the rota who responds to alerts outside working hours), so the weekend pager (the automatic alert that wakes that person) took the hit. We were friendly, so raising it risked the relationship. I prepared the facts, asked permission to talk, said specific words, and agreed a small change we could both check.

Situation and stakes

In three of the last five weekends, a page was traced to a Friday-evening change from this peer. Nobody had complained to him. I was the person on call, so I had standing to speak. The risk: he was senior in tenure, proud of his speed, and likely to hear "you are careless."

Preparation (what I did)

• Listed the three incidents (one per affected weekend) with dates and what each page cost (hours of weekend time), separating what happened from my theory of why.
• Checked my own part: did the team have a "no Friday deploys" norm? No, so he had broken no rule. That changed my goal from "stop doing this" to "agree a norm together."
• Chose a private one-to-one meeting on a Tuesday, not a Friday or in a public channel, and sent a neutral invite: "Want to talk about how we handle late changes and on-call?"

What I actually said

"When changes land after 4pm Friday without a heads-up, on-call finds out from the pager. That happened on three of the last five weekends, including the cache change on the 14th. I'm not saying the changes were wrong. I'm saying I can't be ready for something I don't know about. How do you see it?"

He bristled for a minute ("they were small changes"), so I agreed they were small and asked what would make telling on-call easy. He suggested a one-line note in the on-call channel before merging.

Where the relationship stood afterwards

Slightly awkward for about a week, then normal. We wrote the "tell on-call before a late merge" line into the team's runbook (the shared how-to document). Two weeks later I told him the weekend pages from changes had stopped and thanked him. The follow-up check was simple: count pages tied to late changes each month.

What made it work

• It built psychological safety (people feel safe to speak and be corrected without punishment) by keeping it private, specific, and about the effect on on-call.
• I owned my uncertainty and invited his view.

Pitfalls

Do not deliver it by chat while angry, do not use "always," and do not skip the follow-up; feedback without a check is just a one-time complaint.

Direct answer

I would reply once in the thread to lower the temperature (acknowledge, restate my goal, ask a question), stop the back-and-forth after about two exchanges, and move to a short call or pairing session (two people work through the problem together, live, often sharing a screen). I would bring evidence, listen first, and leave with agreed next steps, then post a summary in the pull request (PR) so the record shows what was decided. The risky issue stays marked as must-fix; the tone around it softens.

In the thread

• Acknowledge the effort and the feeling: "Thanks for the quick reply. I can see this took a lot of work."
• Restate the goal in shared terms: "My worry is the retry loop can double-charge on timeout, and I want us both to be confident it is safe."
• Turn a claim into a question or a reproducible fact: "Could you check what happens when the second call times out? Here is a test that fails on my machine."
• Avoid: all caps, sarcasm, restating the same point louder, public escalation.
• Cut off the thread after two rounds: "This may be easier to talk through. Can we do 15 minutes today?"

The follow-up conversation (a short script)

1. Listen first. Ask "what is your reasoning here?" and repeat back what you heard. Often a constraint I did not know about appears.
2. Bring evidence. A failing test, a log line, a documented incident, a link to the team standard, rather than opinion.
3. Agree next steps and an owner. Fix now, fix in a follow-up PR with a ticket, or decide together the risk is acceptable. Write the decision down.

Worked example

Thread: "Thanks for the detailed explanation. I think the retry is fine for normal cases; I am worried about the timeout case. Here is a test that shows a double write. Could we spend 15 minutes on it?"
Call: they explain the downstream API is idempotent (repeating a request has the same effect as sending it once), which I did not know; I ask them to add that to a code comment and a test. Posted summary: "Agreed on call: retry stays; added comment plus test for the timeout path. Merging after CI (the automated tests that run on every change) passes."

When the disagreement is real

If they are still sure and I still see risk, I ask a third person (the tech lead, the engineer who guides the team's technical decisions, or the owner of that area, the person responsible for that part of the code) for a second opinion on the specific question, rather than escalating the person.

Trade-offs and pitfalls

• Dropping the comment to keep the peace: the risk survives and the next review gets harder.
• Winning the argument and losing the relationship: next time they will hide changes from you.
• Calls leave no record, so a written summary in the PR matters.
• Separate "must fix before merge" from "I would prefer" so the defensive reaction is spent only on what matters.