Lesson 8 / الدرس 8

Reviewing without making an enemy / المراجعة دون صنع عدو

A review is the one place a team routinely tells somebody their work is not right yet. How that is written decides whether it improves the code, and whether the person asks you to review the next one.

المراجعة هي الموضع الوحيد الذي يخبر فيه فريقٌ أحدَهم بانتظام أن عمله ليس صحيحًا بعد. وكيفية كتابتها تقرر أتحسّن الكود، وأيطلب منك الشخص مراجعة التالي.

The same objection can be a useful comment or an insult depending entirely on how it is phrased, and the difference is small and learnable. Almost all of it comes down to reviewing the code rather than the person, and saying what will happen rather than what should have been done.

The same point, four ways

Why didn't you use a transaction here?
     -- "why didn't you" asks them to defend themselves. They will.

This is wrong, it needs a transaction.
     -- a verdict. Nothing to reply to except yes or an argument.

This writes the payment and the booking separately. If the process
dies between them we have money recorded against an unpaid booking.
Worth a transaction?
     -- says what happens, then asks. They can agree, or explain why
        it cannot happen here, and either is a good outcome.

nit: spelling of "recieve" on line 40
     -- labelled small, so they know it is not blocking. Say which
        of your comments are nits and which are not.

The third one does something the first two cannot: it can be answered with information. If the author knows the process cannot die there, they say so and everybody has learned something. The first two can only be answered with compliance or a fight.

What to look for, in order

Does it do what the description says? Not whether it is elegant — whether the thing the ticket wanted now happens.
What happens on the bad path? Empty, huge, slow, unauthorised, twice at once. This is where nearly every real finding is, and it is the reviewer's contribution more than anything about style.
Will somebody understand this in a year? A name that misleads, a decision with no comment behind it. The author will not be there to explain it.
Style, last and briefly. If your team has a formatter, style is not a review topic at all — and if it is not automated, that is the thing worth fixing rather than the file in front of you.

Try it live / جرّب بنفسك

<h1>A review comment, rewritten</h1>
<div class="c bad"><b>As written</b>
  <p></p></div>
<div class="c"><b>What it asks the author to do</b>
  <p></p></div>
<div class="c good"><b>Rewritten</b>
  <p></p></div>
<style>
  body { font-family: system-ui, sans-serif; padding: 1.2rem; line-height: 1.5;
         max-width: 44rem; color: #0f172a; }
  .c { border-left: 5px solid #94a3b8; background: #f8fafc; border-radius: .4rem;
       padding: .7rem 1rem; margin: .7rem 0; }
  .bad { border-left-color: #dc2626; }
  .good { border-left-color: #16a34a; }
  .c p { margin: .35rem 0 0; color: #475569; min-height: 2.4rem; }
</style>

Preview / المعاينة

Check yourself / اختبر نفسك

1. What can "this writes them separately; if the process dies we have money against an unpaid booking" be answered with, that "this is wrong" cannot?

2. Where are nearly all real review findings?

3. You would have written it differently, but it works and is clear. What is that?

Your task / مهمتك

Find a real review comment — one you received, one you wrote, or one from any public pull request. Write it as it was, say what it asks the author to do, then rewrite it so that it says what will happen and can be answered with information.

جد تعليق مراجعة حقيقيًا — تلقيته، أو كتبته، أو من أي طلب سحب عام. اكتبه كما كان، وقل ماذا يطلب من المؤلف، ثم أعد كتابته بحيث يقول ما سيحدث ويمكن إجابته بمعلومة.

The comment as it was actually written التعليق كما كُتب فعلًا
What it asks the author to do — defend, comply, or think ماذا يطلب من المؤلف — أن يدافع أو يمتثل أو يفكر
A rewrite that states the consequence and can be answered with information إعادة كتابة تذكر العاقبة ويمكن إجابتها بمعلومة