Question 4
PR feedback: non-isolated side effects
A junior engineer comes to you saying a senior engineer won't approve their PR because it has too many non-isolated side effects. How do you handle the situation?
Answer outline
Treat it as two problems at once: a technical question about the PR and a communication gap between two engineers. Address both.
Review the PR yourself first and decide whether the concern is valid. If it is, work with the junior to turn the feedback into something concrete and educational instead of a flat rejection. 'Non-isolated side effects' usually means one of three things:
- 1.Shared state mutation: functions that change state other code depends on, in ways a caller wouldn't expect.
- 2.Hidden dependencies: singletons and globals reached from inside a function, so it can't be tested or reasoned about on its own.
- 3.Logic entangled with I/O: business rules mixed into networking or persistence code, so you can't change or test one without the other.
If the feedback is vague or overly strict, coach the senior too. Good review feedback names the specific issue and separates must-fix problems from style preferences.
Use the moment to reinforce team standards. Document what well-decoupled code looks like in your codebase so the same disagreement doesn't come up again.
Principles
- Translate vague feedback into concrete guidance the junior can act on today.
- Coach both sides, since the junior needs direction and the senior needs to communicate more clearly.
- Use the review to teach, and get the PR merged along the way.
- Align on shared standards so the next review doesn't restart the same argument.
