Giving and Receiving Feedback Questions

Direct answer

I would first make sure I understand their objection by restating it and checking the facts, then reply with evidence, not opinion. I concede if their point stands up, hold if I have evidence and the risk is real, and escalate only after a direct conversation has not resolved it, while not letting the PR (pull request, a proposed code change awaiting review) sit.

Step by step

The example used below: the PR makes a payment call that retries on failure. The reviewer worries a retry could charge a customer twice and suggests an idempotency key (a unique token sent with each request so the server recognises a repeat and processes it only once). I think a simpler database rule already prevents it.

1. Pause and restate. "My understanding is that you are worried the retry loop (the code that repeats a failed call) can send duplicate charges. Is that right?" A hard comment is often a real worry stated badly.

2. Separate fact from preference. Is it a claim I can test (a bug, a measurable slowdown) or a style or design preference? Google's published code-review guidance says technical facts and data overrule opinions and personal preferences, and the style guide is the authority on style.

3. Check whether I am wrong. Reproduce their concern with a test. If it fails, concede and thank them.

4. Reply with evidence and the trade-off. "I tried the idempotency-key approach you suggested. It adds a table and a migration (a script that changes the database structure and must be rolled out carefully). My approach avoids that: the payments table already has a unique constraint (a database rule that rejects a second row with the same order id), and here is a test showing a retried call cannot create a second charge. Does that address it?"

   Check the claim against the failure that matters before sending it. A unique constraint only stops a second row in our own table. If the first call timed out after the payment provider had already charged the card, a retry would call the provider again, and no local constraint can stop that. In that case the reviewer is right and the idempotency key (sent to the provider, which many payment providers support) is the correct fix, so I would concede in step 3. My test must therefore simulate a timeout after the provider has processed the charge, not only a repeated local write. My position holds only if the charge record is created first, the provider call carries that record's id, and the provider deduplicates on it.

5. Offer a call if the second reply does not settle it. Text can read harsher than intended.

6. Escalate if still stuck. Google's guidance lists a broader team discussion, a tech lead, a maintainer of the code, or an engineering manager, and says not to leave the change waiting.

Decision table

Close the loop

After a decision, I thank the reviewer, and if I held my position I make sure the decision is recorded in the PR, so the next reader sees why.

What would change my call

A reproducible failing test, a production incident, or a teammate who owns the affected module disagreeing would move me toward conceding.

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

The review comment that worked best was one that named a specific risk, explained the cost in the author's own terms, and offered a small change they could make in the same pull request (PR, a proposed code change awaiting review). I then turned the repeated comment into a team checklist item, so the fix did not depend on me remembering to say it. The story below is a skeleton: swap in your own system and numbers.

Situation

Over about two months, several PRs touching a billing-calculation module came back with follow-up bug fixes within a week of merging. In my review of that module I kept leaving the same kind of comment: a function mixed rounding, currency conversion and database writes, so no test could reach the rounding rule alone. (A bug-fix follow-up within a week of merging is a cheap, countable sign that something slipped through review.)

How I framed it

I did not write "this is messy." I wrote a comment with three parts, in the style of the SBI model (Situation, Behavior, Impact, from the Center for Creative Leadership):

• Situation and behavior: "In apply_discount, rounding happens after the currency conversion and inside the same function that writes the invoice row."
• Impact: "That means we can only test rounding by creating a database row, and last month's rounding bug got through for that reason."
• A small ask: "Could we pull the rounding into a pure function round_amount(amount, currency) (a function whose output depends only on its inputs, with no database or network calls) so we can add table-driven tests (one list of input and expected-output rows, each run through the same test) for the edge cases? Happy to pair on it if useful."

How the author reacted

The author first replied that the extra function felt like over-engineering for a small change. I did not repeat the request. I asked which cases they were most worried about getting wrong, then showed that the two cases they named were exactly the ones the split would make testable. They made the change in that PR and added the tests themselves, which mattered more than my comment did.

Making the change stick

1. I added two lines to the PR template for that module: "Is any money-handling rule testable without a database?" and "Which edge cases did you test?"
2. I stopped leaving the comment by hand. The template question prompted authors, and my reviews could shift to other things.
3. In the team retro (the regular meeting where the team reviews what worked and what did not) I shared the before-and-after of that one PR and asked whether the template question was helping or just noise. One teammate asked to narrow it to money-handling code, and we did.

Result (illustrative numbers, for the shape of the story)

I counted PRs in the module that needed a bug-fix PR within a week: 5 of the 12 merged before the change (about 42%) and 1 of the 11 merged in the two months after (about 9%). That is a small sample, so I treated it as a signal, not proof, and kept counting. The team also now had a written review expectation, which is a process change that outlasted my involvement.

Feedback in a model review

The same pattern applies to a machine-learning model review. A teammate's evaluation script reported high accuracy, but the same users appeared in both the training rows and the test rows. The model can then score well by recognising users it has already seen, so the number overstates how it will do on new users. I commented with the evidence (the count of users present in both sets, from a quick query), asked for a split by user so each person lands wholly in training or wholly in test, and later the team added a "split key" line to the model-review template (a field stating what the data was split by, such as user or date). On the receiving side, I have had a comment of the same shape on my own work, and in both cases the useful parts were the concrete evidence and the template change that followed.

What I would do differently

Ask the author earlier what they were worried about, and agree on the measure (PRs needing a follow-up fix) before the change, not after.

Direct answer

In the moment I listen without defending, thank them, and ask for the specific moment that landed badly. Afterwards I change how I deliver (private first, check intent, plainer wording) without dropping the substance of the feedback, and I repair the relationship with a follow-up that acknowledges the impact and asks how they would like to hear it. Intent and impact are different: I can mean well and still have hurt.

In the moment

• Pause and listen. Do not explain your intent first; that sounds like "you are wrong to be hurt".
• Say thanks and name what you heard: "Thank you for telling me. It sounds like it landed as criticism in front of others, and that stung."
• Ask one specific question: "What was the moment, and what did you hear in it?"
• If I feel defensive, say "I want to think about this properly, can we talk tomorrow?" rather than answering badly.

What I change

1. Venue: negative feedback goes private first unless it is about a team-level process.
2. Structure: use SBI (Situation, Behavior, Impact): a concrete moment, an observable behavior, the effect, with no labels like "careless".
3. Ask before I tell: "Would it be useful to hear some feedback on X?" and "How did that land?" afterward.
4. Tone check on written words: read the message as the recipient would, on a bad day.

Repair

• A short follow-up: "I've thought about what you said. The point I raised still matters, but I should have raised it privately and with an example. I'm sorry it landed that way."
• Do not retract the feedback itself if it was valid; apologise for the delivery.
• Ask: "How would you like me to bring things like this up in future?"
• Then do it. The repair is confirmed by the next few interactions (for example the next two or three times I have critical feedback to give), not by the apology. The number is a rule of thumb, not a measured threshold.

Worked example

I told a peer in a design review, "your error handling is sloppy." Later they said it felt like a public shot. I replied, "Thanks for telling me. What part felt worst?" ("Hearing it in front of the team.") The next day I said: "I should have said this privately. The specific issue: the retry in the payment call swallows failures, which would hide outages. Can we look at it together?" We fixed it, and I now send substantive critiques privately first.

Pitfalls

• "I'm sorry you feel that way" is not an apology.
• Over-correcting into never giving candid feedback again makes things worse for both of you.

Direct answer

I would comment on the claim, not the author, ask for the evidence as a question, say what would change my mind, and mark which comments block approval. I push back when the claim drives a costly or hard-to-undo decision, accept when it is cheap to reverse and the design includes a way to check it, and move to a call when the thread has gone two rounds without converging, when the tone is tightening, or when the disagreement is about meaning (what the claim or the goal actually is) rather than about facts a benchmark could settle.

Example

The design doc says: "The new cache will cut p99 latency (the time within which 99% of requests finish) in half." There is no benchmark.

Comment drafts

• Question about the evidence: "This claim is the main reason for adding the cache, so I want to be sure of it. What workload and data size was this measured on, and what is the baseline? If you have not measured yet, could we add a quick benchmark to the doc, or mark this as a hypothesis with a plan to test it?"
• Say what would settle it: "A comparison of cache-on vs cache-off on a replay of last week's traffic (recorded real requests sent through both versions) would convince me."
• Label severity: start with "Blocking:" for the claim, and "Nit:" or "Optional:" for minor edits, so the author knows what must change.
• Acknowledge what is good: "The invalidation section (how stale cached entries get removed) is clear, and the rollout plan is thoughtful."

Push back or accept

When to move to a call

1. After two rounds of replies that still disagree.
2. When the thread is getting longer or the tone is tightening, since text hides intent.
3. When the author is likely to be discouraged by a stack of comments.
4. When the disagreement is about meaning, such as what "latency" or "fast enough" refers to, not about a fact a measurement could settle. A call clears up definitions quickly, while more text rarely does.

Accepting an incidental claim without a check is safe only if the thread says it is unverified, so nobody later builds on it as if it were measured.

After the call, write the outcome back into the doc ("agreed to benchmark on replayed traffic, owner Sam, due Friday") so the decision is on record.

Written feedback on a research-style draft

The same structure fits a colleague's paper draft: a short summary of what you understood the claim to be, then strengths (specific), then concerns ordered by importance (for example: the baseline is weak, the claim is stronger than the experiment supports), each with a suggested check, in a respectful tone. A concern written out: "The comparison is against a model with default settings, so the gain may come from tuning, not the new method. Could you add a baseline that gets the same tuning effort? If the gap holds, the claim is much stronger." Starting with the summary shows the author whether you understood them, which often removes half the disagreement.