今日推荐开源项目：《这是什么？是代码 Code Surfer》传送门：GitHub链接
推荐理由：一个 React 组件，能够帮助你突出你的代码，如果你有一大堆代码要说明，为什么要把它们丢在编辑器里，然后用你的鼠标去划出重要的那部分？试试这个吧，这可比鼠标划过去简单而管用得多。
今日推荐英文原文：《Passing Code Reviews Fast》作者：Jackie Tung
Passing Code Reviews Fast
Coming from being a code monkey on Wall Street trading desks, I recall the culture shock during code reviews in my first job in Silicon Valley. Why are we nitpicking indentation and variable names? Who gives a sh*t about being Pythonic? Unit tests? Pfft! Is a LGTM from me, to me not enough?
Four years later, I’m fully adjusted. I get that having code reviews probably is a sensible idea. This article won’t debate the merits of code review culture, nor recommend tooling of any kind. Instead, I will focus on human aspects to sustainably getting to “ship it!“ as fast as possible.
Specific reviewers usually have no obligation to look at your code at all. Even if they do look, timeframes are not guaranteed.
In short, each code review performed is a favor granted by the reviewer. So ask nicely. For example:
- Never take eventual code acceptance for granted. It’s disrespectful to diligent reviewers. Blind accepts by indifferent reviewers defeats the whole purpose of code review.
- Correctly frame urgent requests as your own fault for not planning ahead, not the fault of reviewers being slow. It’s up to you to address all feedback as fast as you can.
Accepting feedback often means extra work. This leads to a reflex to resist actionable feedback — I have seen it in myself and others. This typically manifests as grasping at weak technical arguments to avoid making changes.
In the extreme, this can lead to people not wanting to review your code at all. It is a losing proposition: They cannot accept code without reasonable fixes; but you have a reputation for being unwilling to listen.
Take a step back, fight the inertia, and be open to feedback. It might be good for you.
Text-only communication is terrible at expressing tone and emotions. It is common for review feedback messages to be taken the wrong way.
Once an issue escalates beyond the purely objective (e.g. A is better than B, because 1+1=2), just walk over and have a real conversation. Communication bandwidth is much better face-to-face for most people anyway.
On the off-chance that you think you are being trolled — that nothing you ever code seems good enough — what are you to do? First you should establish that the reviewer is indeed being unreasonable. What if your code really is crap? While the reviewer’s attitude is uncalled for, perhaps you need to improve your coding skills also.
You need a second opinion. Lay out the evidence for your manager or peer. They can let you know whether it’s you or them.
What doesn’t kill you makes you stronger — So relax: getting trolled in code reviews from time to time probably just makes you a better professional software engineer.
Perfect is the Enemy of Good
There is always a smarter way, or an extra abstraction that makes the code more generic, scalable, extensible, or harder-to-read, etc. Some reviewers enjoy pointing this out.
But code needs to be shipped to provide value today. What’s the best way to handle this? Perfection is seldom required and never budgeted for. Perhaps polishing this specific piece of code to handle 1000x load is not an efficient use of your development time.
Here is one reasonable way to handle this:
- Thank the reviewer for the great feedback.
- Explain the immediate value provided by the baseline code change.
- Add a TODO documenting the suggested improvement as an inline comment.
- Add a task to your team’s project management system (cc. the reviewer).
- Allow management to prioritize this accordingly.
Obviously, spamming this strategy to deflect every suggestion won’t make you popular. When appropriately used however, this is graceful way to break an impasse for all parties. The reviewer feels like his smart idea is acknowledged (and credited in the TODO); you get to ship; users get value now. Win, win, win!
Perhaps you made a conscious coding decision to take certain shortcuts rather than reach for perfection. Write a comment right then and there! Explain the justifications and preemptively add a TODO. This saves time during code review, and lets reviewers (and maintainers) know that you were thoughtful and deliberate about your code.
The Follow-Up Change
Suppose you agree with a suggestion for a big improvement, but it involves lots of additional code. Should you do it right now? If the baseline code is functional and correct, it might not be a good idea. Other reviewers might have already spent time reviewing and are satisfied with the baseline code. In your subconscious rush to rewrite the algorithm and inject more code, what’s to say that no new bugs are introduced? If there is no obvious harm done to push the code as is, then just do so.
But you must make a follow-up change. This is similar to the TODO prioritization strategy earlier, except that you work on the improvement immediately after the baseline code passes review and is pushed to trunk.
Make sure you follow through though, or your reputation goes to sh*t.