Code review — কী দেখবেন, কীভাবে মন্তব্য করবেন, linter আর static analysis

১ · Code review কেন — ভুল ধরা থেকে জ্ঞান ভাগাভাগি পর্যন্ত

পর্ব ১৬-এ (Pull request আর fork) দেখেছেন একটি pull request কীভাবে খোলা হয়, review-এর তিন রকম সিদ্ধান্ত কী, আর মন্তব্য পেয়ে branch কীভাবে হালনাগাদ হয়। সেটি ছিল যন্ত্রপাতি। এই পর্বে প্রশ্ন হলো কাজটি: reviewer আসলে কী খুঁজবেন, কোন ক্রমে খুঁজবেন, কী ভাষায় বলবেন, আর কোন কাজ মানুষের বদলে যন্ত্রকে দেবেন।

একটি দল কেন review করে, তার পাঁচটি প্রধান কারণ:

আজকের বেশিরভাগ দল যা করে, গবেষকেরা তার নাম দিয়েছেন modern code review: হালকা, tool-ভিত্তিক (PR-এর পাতা), সভা ছাড়া, নিজের সময়ে (asynchronous), আর প্রায় প্রতিটি পরিবর্তনে। এই ধারার সবচেয়ে উদ্ধৃত গবেষণাগুলোর একটি Alberto Bacchelli আর Christian Bird-এর “Expectations, Outcomes, and Challenges of Modern Code Review” (ICSE ২০১৩), Microsoft-এর ডেভেলপারদের ওপর। আমাদের বোঝামতে তাঁদের মূল পর্যবেক্ষণ: ডেভেলপাররা review-এর প্রধান উদ্দেশ্য হিসেবে ত্রুটি খোঁজাকে বলেন, কিন্তু বাস্তবে পাওয়া মন্তব্যের বড় অংশ ছিল কোডের মান আর পড়ার সুবিধা নিয়ে, আর জ্ঞান স্থানান্তর, দলের সচেতনতা (কে কী বদলাচ্ছে) ও বিকল্প সমাধান খোঁজা ছিল বড় লাভ। সবচেয়ে বড় বাধা ছিল পরিবর্তনটি বোঝা। এক কোম্পানির গবেষণা সব দলে হুবহু খাটবে না, তবে দিকটি পরের গবেষণাতেও মিলেছে।

Eric Raymond-এর প্রবন্ধ The Cathedral and the Bazaar (১৯৯৭; বই ১৯৯৯)-এর “Linus’s law” বলে: “given enough eyeballs, all bugs are shallow”. কিন্তু চোখ থাকা আর চোখ পড়া এক নয় — ভাবনার প্রশ্ন ০২-এ দেখবেন।

২ · Review-এর পাইপলাইন — লেখক, যন্ত্র, মানুষ, তারপর merge

ভালো দলে একটি পরিবর্তন চারটি ছাঁকনি পার হয়, আর প্রতিটির ব্যর্থতা লেখকের কাছেই ফিরে আসে। কোন ছাঁকনি কী ধরবে, তা আগে থেকে ভাগ করা থাকলে কেউ অন্যের কাজ নকল করেন না — মানুষ indentation গোনেন না, আর যন্ত্রকে নকশার প্রশ্ন জিজ্ঞেস করা হয় না।

1. লেখকের নিজের review. নিজের diff পড়া, test চালানো; একটি pre-commit hook commit-এর মুহূর্তেই formatter আর linter চালায় (Git hook পর্ব ৫৬-এ)।
2. যন্ত্রের check. Push হলেই CI একই check আবার চালায়, কারণ সবার কম্পিউটারে hook না-ও থাকতে পারে। ফল PR-এ status check; branch protection লাল check-এ merge আটকায় (pipeline পর্ব ৫৮-এ)।
3. মানুষের review. সবুজ check-এর পরে, শুধু সেই প্রশ্নে যা যন্ত্র পারে না।
4. Merge. Approval, সব check আর thread মীমাংসার পরে (পদ্ধতি পর্ব ১৬-এ)।

৩ · যন্ত্রের review — formatter, linter, static analysis আর type checker

চার ধরনের tool, চার রকম কাজ। নামগুলো কখনো মিশে যায় (অনেক linter এখন formatting-ও করে), তবু ভাগটি মাথায় রাখা কাজের:

“Lint” নামটি এসেছে ১৯৭০-এর দশকের শেষে Bell Labs-এ Stephen C. Johnson-এর লেখা C-এর একটি checker থেকে — কাপড়ে লেগে থাকা আঁশ (lint) ঝাড়ার মতো। কোন tool সবচেয়ে প্রচলিত, তা বছর বছর বদলায়; নামের চেয়ে ভাগটি গুরুত্বপূর্ণ।

Linter ভেতরে কীভাবে কাজ করে — একটি খুদে linter

বেশিরভাগ linter কোডকে লেখা হিসেবে নয়, গাছ হিসেবে পড়ে — abstract syntax tree (AST). Python-এর standard library-র ast module ঠিক এই গাছ দেয়। নিচের ৩৫ লাইনের খেলনা linter চারটি নিয়ম মানে: (W1) list বা dict-এর মতো বদলযোগ্য default argument, (W2) bare except, (W3) রেখে দেওয়া print, (W4) মান বসানো হয়েছে কিন্তু কখনো পড়া হয়নি এমন variable.

import ast
# Toy linter: 4 rules on Python's own syntax tree (real tools: Ruff, flake8, pylint).
SOURCE = '''def add_fee(order, items=[]):
    total = 0
    discount = 5
    for it in items:
        total += it["price"]
    print("debug total", total)
    try:
        order.save(total)
    except:
        pass
    return total
'''
def lint(src):
    out = []
    for fn in ast.walk(ast.parse(src)):
        if not isinstance(fn, ast.FunctionDef): continue
        for d in fn.args.defaults:                                  # W1
            if isinstance(d, (ast.List, ast.Dict, ast.Set)):
                out.append((d.lineno, "W1 mutable default argument"))
        stored, used = {}, set()
        for n in ast.walk(fn):
            if isinstance(n, ast.Name):
                if isinstance(n.ctx, ast.Store): stored.setdefault(n.id, n.lineno)
                else: used.add(n.id)
            elif isinstance(n, ast.ExceptHandler) and n.type is None:  # W2
                out.append((n.lineno, "W2 bare except"))
            elif isinstance(n, ast.Call) and getattr(n.func, "id", "") == "print":
                out.append((n.lineno, "W3 print left in"))           # W3
        out += [(ln, f"W4 unused variable '{v}'")                      # W4
                for v, ln in stored.items() if v not in used]
    return sorted(out)
for line, msg in lint(SOURCE):
    print(f"line {line}: {msg}")

line 1: W1 mutable default argument
line 3: W4 unused variable 'discount'
line 6: W3 print left in
line 9: W2 bare except

Static analysis কী বাড়তি দেয় — আর তার দাম

Linter সাধারণত এক ফাইলের ধাঁচ দেখে। Static analysis tool পুরো প্রোগ্রাম জুড়ে data-র পথ অনুসরণ করতে পারে: “এই form-এর মান কোনো যাচাই ছাড়াই ঘুরে ঘুরে একটি SQL query-তে ঢুকছে” — একে বলে taint analysis. CodeQL কোডকে database-এর মতো query করতে দেয়; SonarQube-ধাঁচের platform নিয়ম, জটিলতা আর পুনরাবৃত্তির হিসাব এক ড্যাশবোর্ডে দেখায় — সুবিধা সংস্করণ আর license-ভেদে আলাদা।

দাম হলো false positive: tool এমন জায়গাকে সমস্যা বলে, যা আসলে সমস্যা নয়। বেশি হলে মানুষ সতর্কবার্তা পড়াই ছেড়ে দেন। তাই অভিজ্ঞ দল অল্প নিয়মে শুরু করে, পুরোনো কোডের হাজারো সতর্কবার্তা একবারে না মিটিয়ে শুধু নতুন কোডে নিয়ম বাধ্যতামূলক করে (baseline), আর কোনো নিয়ম চুপ করালে কারণ লেখে — legacy কোডে ধাপে ধাপে উন্নতির কৌশল পর্ব ৪৩-এ (Technical debt, legacy code আর maintenance).

Commit-এর মুহূর্তে — pre-commit hook

Python জগতে pre-commit নামের জনপ্রিয় framework একটি YAML ফাইল পড়ে commit-এর আগে tool চালায় (JS জগতে husky আর lint-staged একই কাজ করে)। নিচে rev-এ লেখার সময়ের সংস্করণ; ব্যবহারের আগে সর্বশেষ সংস্করণ আর hook-এর নাম documentation-এ মিলিয়ে নিন:

repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.6.9
    hooks:
      - id: ruff          # linter; --fix applies safe automatic fixes
        args: [--fix]
      - id: ruff-format   # formatter
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: detect-private-key
      - id: check-added-large-files

শেষের দুটি hook ভুল করে private key বা বিশাল ফাইল commit হওয়া আটকায়। তবে hook লেখকের কম্পিউটারে চলে আর git commit --no-verify দিয়ে পাশ কাটানো যায় — এটি সুবিধা, নিশ্চয়তা নয়। নিশ্চয়তা আসে CI-তে একই check আর branch protection থেকে।

৪ · কী দেখবেন, কোন ক্রমে — একটি diff ধাপে ধাপে

বাড়ি তৈরির নকশা যাচাইয়ের কথা ভাবুন। প্রকৌশলী প্রথমে দেখেন ভিত আর কলাম ঠিক আছে কি না; তারপর ঘরের বিন্যাস; একেবারে শেষে জানালার রং। আগে রং নিয়ে আধঘণ্টা কথা বলে পরে দেখা গেল কলাম ভুল — তাহলে রং নিয়ে আলোচনাটাই নষ্ট। Review-ও একই: বড় প্রশ্ন আগে, কারণ বড় প্রশ্নের উত্তর ছোটগুলোকে অপ্রাসঙ্গিক করে দিতে পারে।

1. প্রসঙ্গ: PR-এর বিবরণ আর linked issue — কী সমস্যা সমাধান হচ্ছে? পরিবর্তনটি আদৌ দরকার কি?
2. সঠিকতা আর test: কোডটি যা বলছে তা করে কি না; সীমার মান (০, ঋণাত্মক, খালি list, সর্বোচ্চ); test আছে কি না আর সেগুলো আসলেই আচরণ যাচাই করে কি না।
3. নকশা আর architecture-এ মানানসই: কোডটি ঠিক স্তরে বসেছে কি না — যেমন domain-এর নিয়মের ভেতরে সরাসরি database ডাকা (পর্ব ৩৮ Software architecture আর পর্ব ৪০ Clean আর hexagonal architecture).
4. পড়ার সুবিধা আর নাম: ছয় মাস পরে নতুন কেউ বুঝবেন কি? Function একটি কাজ করে কি না (পর্ব ৪১ Clean code, পর্ব ৪২ Code smell আর refactoring).
5. নিরাপত্তা: বাইরের ইনপুট, secret, কে কী করতে পারবেন (অংশ ৫)।
6. Performance — যেখানে প্রাসঙ্গিক: loop-এর ভেতরে query, অপ্রয়োজনীয় বড় কপি; কিন্তু মাপা ছাড়া অনুমানে micro-optimisation চাওয়া নয়।
7. Error handling: ব্যর্থ হলে কী হয়? Error চাপা পড়ছে কি?
8. নথি: public interface বদলালে docstring, README, changelog (পর্ব ১৮); ডাকে এমন অন্য কোড ভাঙছে কি না।

এবার একটি কাল্পনিক অনলাইন বাজার অ্যাপের PR. লেখক coupon-এর হিসাবে “ন্যূনতম কেনাকাটা” শর্ত যোগ করেছেন। আসল git diff:

$ git diff -- coupon.py
diff --git a/coupon.py b/coupon.py
index 1073cfa..b77f764 100644
--- a/coupon.py
+++ b/coupon.py
@@ -1,5 +1,14 @@
-def apply_coupon(cart, code):
-    coupon = coupons.find(code)
+def apply_coupon(cart, code, user):
+    print("coupon", code, user.phone)
+    sql = f"SELECT * FROM coupons WHERE code = '{code}'"
+    coupon = db.fetch_one(sql)
     if coupon is None:
         return cart.total
-    return cart.total - coupon.amount
+    try:
+        if coupon["min_total"] > cart.total:
+            return cart.total
+    except:
+        pass
+    for item in cart.items:
+        db.log_coupon_view(item.id, code)
+    return cart.total - coupon["amount"]

নিচের stepper-এ নতুন ফাইলটি (ডানে লাইন নম্বর) ওপরের ক্রমে পড়া হচ্ছে। প্রতিটি ধাপে কোন লাইন দেখছেন, কী পেলেন, আর মন্তব্যটি কোন ধরনের:

1def apply_coupon(cart, code, user):2    print("coupon", code, user.phone)3    sql = f"SELECT * FROM coupons WHERE code = '{code}'"4    coupon = db.fetch_one(sql)5    if coupon is None:6        return cart.total7    try:8        if coupon["min_total"] > cart.total:9            return cart.total10    except:11        pass12    for item in cart.items:13        db.log_coupon_view(item.id, code)14    return cart.total - coupon["amount"]

লক্ষ করুন, এর অন্তত দুটি — bare except আর রেখে দেওয়া print — অংশ ৩-এর linter-ই ধরত, আর SQL-এর লেখায় ইনপুট বসানো অনেক SAST tool ধরে। যন্ত্র আগে চললে reviewer-এর খাতা ছোট হতো, আর মনোযোগ যেত ঋণাত্মক মোট আর নকশার প্রশ্নে।

Reviewer-এর চেকলিস্ট

নিচের code review checklist একটি শুরুর খসড়া; দলের অভিজ্ঞতা অনুযায়ী বদলান, আর যে প্রশ্নের উত্তর যন্ত্র দিতে পারে, সেটি তালিকা থেকে সরিয়ে tool-এ দিন। লম্বা তালিকা কেউ পড়ে না — দশ-বারোটি প্রশ্নই যথেষ্ট।

• PR-এর বিবরণ পড়ে বুঝেছি কী আর কেন; পরিবর্তনটি একটি কাজের মধ্যে সীমিত
• সীমার মান (০, ঋণাত্মক, খালি, সর্বোচ্চ, null) আর ব্যর্থতার পথ ভেবে দেখেছি
• নতুন আচরণের test আছে, আর ভুল কোডে test-টি সত্যিই fail করবে
• কোড ঠিক স্তরে বসেছে; নতুন dependency বা নতুন ধাঁচ আনার কারণ আছে
• নাম আর গঠন দেখে উদ্দেশ্য বোঝা যায়; জটিল জায়গায় “কেন”-এর মন্তব্য আছে
• বাইরের ইনপুট যাচাই হচ্ছে; SQL, shell বা HTML-এ ইনপুট সরাসরি জোড়া নেই
• কোনো secret, token বা ব্যক্তিগত তথ্য কোডে বা log-এ নেই
• অনুমতির যাচাই আছে: এই ব্যবহারকারী কি এই কাজটি করতে পারেন?
• Loop-এ query, বড় data-র অপ্রয়োজনীয় কপি নেই — যেখানে আকার বড় হতে পারে
• Error চাপা পড়ছে না; ব্যবহারকারী আর log দুই জায়গাতেই অর্থপূর্ণ বার্তা
• Public interface বদলালে নথি, changelog আর পুরোনো caller-এর কথা ভাবা হয়েছে
• অন্তত একটি ভালো দিক খুঁজে লিখেছি

৫ · নিরাপত্তার চোখে review — ইনপুট, secret আর অনুমতি

সব reviewer নিরাপত্তা-বিশেষজ্ঞ নন, হওয়ার দরকারও নেই। কিন্তু কয়েকটি প্রশ্ন প্রতিটি review-এ তোলা যায়, আর এগুলোই বেশিরভাগ সাধারণ ভুল ধরে। সবকিছুই এখানে প্রতিরক্ষার দিক থেকে — নিজের কোডকে শক্ত করার জন্য।

এই প্রশ্নগুলোর পেছনে গোছানো জ্ঞানভাণ্ডার আছে: OWASP (Open Worldwide Application Security Project)-এর Code Review Guide আর সাধারণ ঝুঁকির তালিকা “OWASP Top 10”, আর MITRE-এর CWE তালিকা, যেখানে প্রতিটি দুর্বলতার ধরনের একটি নম্বর আছে (যেমন SQL injection-এর জন্য CWE-89). Python আর JavaScript-এ নির্দিষ্ট উদাহরণসহ আরও জানতে Python-এ নিরাপদ কোড আর JavaScript-এ web নিরাপত্তা পর্ব দেখুন; বড় system-এর নকশার দিক System Design-এর নিরাপত্তা পর্বে।

৬ · কীভাবে মন্তব্য করবেন — সদয়, নির্দিষ্ট, কারণসহ, আর অবস্থা স্পষ্ট

একই সমস্যার কথা দুভাবে বলা যায়, আর ফল হয় সম্পূর্ণ আলাদা। দর্জির দোকানে ভাবুন: “জামাটা খারাপ হয়েছে” শুনে দর্জি কিছুই ঠিক করতে পারবেন না। “ডান হাতা বাঁ হাতার চেয়ে প্রায় এক ইঞ্চি লম্বা — মাপের কাগজে দুটোই ২২ ইঞ্চি লেখা ছিল” শুনলে তিনি জানেন কী, কেন, আর কোথায় ঠিক করতে হবে। ভালো review মন্তব্যের ছয়টি গুণ:

• কোড নিয়ে, মানুষ নিয়ে নয়: “আপনি ভুল করেছেন” নয়, “এই loop খালি list-এ …”।
• নির্দিষ্ট: কোন লাইন, কোন ইনপুটে, কী ঘটে।
• কারণসহ: bug, নিরাপত্তা, দলের নিয়ম, নাকি রুচি? কারণ জানলে লেখক পরের বার নিজেই ধরবেন।
• কাজে লাগানো যায়: একটি প্রস্তাব, উদাহরণ বা প্রশ্ন।
• অবস্থা স্পষ্ট: merge আটকাবে (blocking), ঐচ্ছিক (nit), নাকি শুধু প্রশ্ন?
• আদেশের বদলে প্রশ্ন, আর প্রশংসাও: “এখানে None এলে কী হবে?” লেখককে ভাবায়, আর হয়তো এমন কারণ বেরোয় যা reviewer জানতেন না। ভালো কাজ চোখে পড়লে লিখুন — কোন ধাঁচ চালিয়ে যাওয়া উচিত, সেটিও শেখা।

অবস্থা স্পষ্ট করার একটি প্রকাশিত প্রস্তাব হলো Conventional Comments (conventionalcomments.org; আমাদের জানামতে Paul Slaughter-এর প্রস্তাব)। এতে প্রতিটি মন্তব্য শুরু হয় একটি লেবেলে — যেমন praise, nitpick, suggestion, issue, question, thought, todo — আর ঐচ্ছিক “decoration” বন্ধনীতে, যেমন (blocking), (non-blocking). গঠন: <label> (<decoration>): <subject>, তারপর দরকারে আলোচনা। এটি একটি সম্প্রদায়ের প্রস্তাব, কোনো মান (standard) নয়; অনেক দল নিজের ছোট রূপ বানায় (শুধু nit: আর blocking:)। যেভাবেই হোক, blocking comment কোনটি, তা পাঠকের অনুমানের ওপর ছেড়ে দেবেন না।

নিচের খেলনা প্রোগ্রামটি কয়েকটি মন্তব্য পরীক্ষা করে: কড়া বা তাচ্ছিল্যের শব্দ, লেবেল নেই, blocking কি না অস্পষ্ট, আর কারণ নেই। এটি শুধু শব্দ মেলায় — ভাষা বোঝে না, তা নিজেই দেখবেন:

// Toy review-comment checker: keyword rules only, no real language understanding.
const HARSH = ["stupid", "terrible", "obviously", "just", "lazy", "why would you"];
const LABELS = ["issue", "suggestion", "nitpick", "question", "praise", "thought"];
const NEEDS_REASON = ["issue", "suggestion"];          // nitpick/question/praise: optional
const REASON = ["because", "so that", "otherwise", "since", "risk"];

function check(text) {
  const t = text.toLowerCase(), notes = [];
  const label = LABELS.find(l => t.startsWith(l));
  const harsh = HARSH.filter(w => new RegExp("\\b" + w + "\\b").test(t));
  if (harsh.length) notes.push("harsh/dismissive words: " + harsh.join(", "));
  if (!label) notes.push("no label: is this blocking, a nit or a question?");
  if (NEEDS_REASON.includes(label)) {
    if (!/\((non-)?blocking\)/.test(t)) notes.push("blocking status unclear");
    if (!REASON.some(r => t.includes(r))) notes.push("no reason (why does it matter?)");
  }
  return { label: label || "-", notes };
}

const comments = [
  "This is terrible. Just use a map.",
  "issue (blocking): amount can be negative because the form allows '-'. Add a check?",
  "suggestion: extract the fee rule into a function.",
  "nitpick (non-blocking): rename tmp to pendingOrders.",
  "Why would you call the DB inside the loop?",
  "question: can this list be empty? If so, line 14 throws.",
  "praise: this just works with the old API, nice!",
];
for (const c of comments) {
  const r = check(c);
  console.log((r.notes.length ? "FLAG " : "ok   ") + r.label.padEnd(11) + c.slice(0, 34) + "…");
  r.notes.forEach(n => console.log("       - " + n));
}
console.log("\nTemplate: <label> (<blocking|non-blocking>): <what you saw>, <why it matters>. <suggestion or question>");

FLAG -          This is terrible. Just use a map.…
       - harsh/dismissive words: terrible, just
       - no label: is this blocking, a nit or a question?
ok   issue      issue (blocking): amount can be ne…
FLAG suggestion suggestion: extract the fee rule i…
       - blocking status unclear
       - no reason (why does it matter?)
ok   nitpick    nitpick (non-blocking): rename tmp…
FLAG -          Why would you call the DB inside t…
       - harsh/dismissive words: why would you
       - no label: is this blocking, a nit or a question?
ok   question   question: can this list be empty? …
FLAG praise     praise: this just works with the o…
       - harsh/dismissive words: just

Template: <label> (<blocking|non-blocking>): <what you saw>, <why it matters>. <suggestion or question>

এবার একটি কড়া মন্তব্য ধাপে ধাপে কাজের মন্তব্যে বদলানো দেখুন। বিষয়: অংশ ৪-এর coupon.py-এর লাইন ১২–১৩, যেখানে প্রতিটি item-এর জন্য একটি query.

Why would you query the DB inside a loop??
This is obviously wrong. Fix it.

This loop runs one DB query per cart item.

This loop runs one DB query per cart item
(db.log_coupon_view on line 13). A cart with
40 items makes 40 extra queries per checkout.

This loop runs one DB query per cart item
(line 13). A 40-item cart makes 40 extra
queries per checkout, because each call is a
separate round trip; during sale rushes that
adds up on the checkout path.

... adds up on the checkout path.
Could we log all item ids in one call, e.g.
db.log_coupon_views([i.id for i in cart.items], code)?
Or is there a reason it must be per item?

suggestion (non-blocking): this loop runs one DB
query per cart item (line 13). A 40-item cart
makes 40 extra queries per checkout, because each
call is a separate round trip. Could we log all
ids in one call, e.g. db.log_coupon_views(ids, code)?
Happy to merge without it if a follow-up issue exists.

৭ · লেখকের দায়িত্ব আর feedback নেওয়া — fixup commit দিয়ে সাড়া

Review দুজনের কাজ। reviewer-এর সময় বাঁচানোর সবচেয়ে বড় সুযোগ থাকে লেখকের হাতে — PR খোলার আগে।

লেখক যা করবেন

• Self-review: PR খুলে platform-এর diff পাতায় নিজের পরিবর্তন পুরোটা পড়ুন, যেন অন্যের কোড — debug-এর print, মন্তব্য করে রাখা কোড, অপ্রাসঙ্গিক ফাইল এখানেই ধরা পড়ে।
• ছোট, এক কাজের PR (পর্ব ১৬); কেন জরুরি, অংশ ৮-এর মডেলে দেখবেন।
• প্রসঙ্গ: কী, কেন, কোন বিকল্প বাদ দিলেন, reviewer কোথায় বেশি মন দেবেন (“refund.py-এর শর্তটি মূল; বাকিটা নাম বদল”); UI বদলালে আগে-পরের screenshot.
• Test নতুন আচরণের, আর হাতে কীভাবে পরীক্ষা করলেন। “Locally works” প্রমাণ নয়।
• CI সবুজ করে তবে review চাওয়া — লাল check নিয়ে কাউকে ডাকা মানে তাঁর সময়কে যন্ত্রের সমান ধরা।
• যান্ত্রিক আর অর্থপূর্ণ বদল আলাদা commit-এ; reviewer git diff --color-moved বা -w দিয়ে যান্ত্রিক অংশ দ্রুত পার করেন।

Feedback নেবেন কীভাবে

নিজের কোডে মন্তব্যের তালিকা দেখলে খারাপ লাগা স্বাভাবিক — ক্রিকেটে ক্যাচ ফসকানোর পরে অধিনায়কের দিকে তাকানোর মতো। কয়েকটি অভ্যাস সাহায্য করে:

• মন্তব্য কোডের ওপর, আপনার ওপর নয় — আর প্রতিটি মন্তব্য একজন সহকর্মীর দেওয়া সময়।
• প্রতিটি thread-এর উত্তর দিন: “ঠিক করেছি, df0e077”, বা কারণসহ দ্বিমত। যুক্তি দিয়ে দ্বিমত অভদ্রতা নয়।
• তর্ক তিন-চার বার্তা পেরোলে ছোট একটা call, তারপর সিদ্ধান্তটি thread-এ লিখে রাখা।
• একই মন্তব্য বারবার পেলে নিজের চেকলিস্টে যোগ করুন, বা একটি linter নিয়ম প্রস্তাব করুন।

এখন একটি কাল্পনিক refund-এর PR. Branch fix/refund-check-এ দুটি commit: 322edad “Reject refunds larger than the payment” আর 8346907 “Add refund tests”. Reviewer তিনটি মন্তব্য দিয়েছেন। লেখক কীভাবে সাড়া দেন — দল মন্তব্যের সময় history-বদল চায় না, শুধু merge-এর আগে একবার পরিষ্কার (পর্ব ১৫ Rebase-এর autosquash):

একই ধাপগুলোর আসল Git আউটপুট (Git ২.৫৩-এ; লেখক Rahim, সময় স্থির রাখা — তাই hash পুনরুৎপাদনযোগ্য; remote একটি স্থানীয় bare repository). Interactive rebase-এ editor খুললে Git নিজেই fixup লাইনগুলো সঠিক জায়গায় সাজিয়ে দেয়; তালিকা না বদলে সংরক্ষণ করে বন্ধ করলেই চলে:

$ git commit --fixup=322edad
[fix/refund-check df0e077] fixup! Reject refunds larger than the payment
 1 file changed, 2 insertions(+), 2 deletions(-)
$ git commit --fixup=8346907
[fix/refund-check 3b7cb93] fixup! Add refund tests
 1 file changed, 5 insertions(+), 1 deletion(-)
$ git push
To ../origin.git
   8346907..3b7cb93  fix/refund-check -> fix/refund-check
$ git log --oneline main..
3b7cb93 fixup! Add refund tests
df0e077 fixup! Reject refunds larger than the payment
8346907 Add refund tests
322edad Reject refunds larger than the payment
$ git rebase -i --autosquash main
Successfully rebased and updated refs/heads/fix/refund-check.
$ git log --oneline main..
6ea1995 Add refund tests
af51301 Reject refunds larger than the payment
$ git push --force-with-lease
To ../origin.git
 + 3b7cb93...6ea1995 fix/refund-check -> fix/refund-check (forced update)
$ git range-diff --creation-factor=100 main 8346907 HEAD
1:  322edad ! 1:  af51301 Reject refunds larger than the payment
    @@ refund.py
     @@
      def refund(order, amount):
          """Give money back to the customer."""
    -+    if amount > order.paid:
    -+        raise ValueError("refund exceeds payment")
    ++    if amount <= 0 or amount > order.paid:
    ++        raise ValueError("refund must be between 0 and the payment")
          order.balance -= amount
          order.history.append(("refund", amount))
          return order.balance
2:  8346907 ! 2:  6ea1995 Add refund tests
    @@ test_refund.py (new)
     +    paid, balance, history = 500, 0, []
     +
     +class RefundTest(unittest.TestCase):
    -+    def test_too_large(self):
    ++    def test_refund_larger_than_paid_is_rejected(self):
     +        with self.assertRaises(ValueError):
     +            refund(Order(), 600)
    ++
    ++    def test_negative_refund_is_rejected(self):
    ++        with self.assertRaises(ValueError):
    ++            refund(Order(), -50)

৮ · সময়, চাপ আর অগ্রাধিকার — turnaround, review load আর ঝুঁকির হিসাব

Review-এর গুণমান পরিস্থিতির ওপরও নির্ভর করে। SmartBear-এর Jason Cohen-এর বই Best Kept Secrets of Peer Code Review (২০০৬)-এ বর্ণিত Cisco-র সমীক্ষার সারকথা (পর্ব ১৬-এ সংক্ষেপে দেখেছেন): একবারে বেশি লাইন, খুব দ্রুত পড়া আর একটানা দীর্ঘ review — তিনটিই ত্রুটি খুঁজে পাওয়ার হার কমায়। সংখ্যাগুলো এক পরিবেশের, তাই দিচ্ছি না।

অন্য দিক হলো review turnaround — অনুরোধ থেকে প্রথম সাড়া পর্যন্ত সময়। Google-এর প্রকাশ্য engineering practices নির্দেশিকায় (“Speed of Code Reviews”) আমাদের জানামতে বলা আছে, সাড়া দিতে সর্বোচ্চ এক কর্মদিবস লাগা উচিত। একই নির্দেশিকা বলে: পরিবর্তন নিখুঁত না হলেও সামগ্রিকভাবে কোডের স্বাস্থ্য স্পষ্টভাবে ভালো করলে approve করা উচিত — নিখুঁত কোড নেই, আছে আরও ভালো কোড।

অপেক্ষা কেন এত বাড়ে, তা queue-র গণিত — পর্ব ৬-এর (Kanban আর lean) Little’s law আর WIP. পর্ব ১৬-এর মডেলে সব PR সমান আকারের ছিল; এবার মিশ্র আকার আর reviewer-এর সংখ্যা বদলে দেখুন। অনুমানগুলো বানানো, শুরুতেই লেখা:

// Review load model. ASSUMPTIONS (invented for the model, not measured data):
//  A1 PRs arrive at random, on average one every 2.25 working hours, for 2000 hours
//  A2 PR size: 60% small (20-100 lines), 30% medium (100-400), 10% large (400-1200)
//  A3 one review takes 0.2 h + lines/250 h; FIFO queue; R reviewers, each one PR at a time
//  A4 a review asks for another round with probability min(0.7, lines/1000);
//     the author fixes in 1 h, then the PR joins the queue again
function mulberry32(a) { return () => { a |= 0; a = a + 0x6D2B79F5 | 0; let t = Math.imul(a ^ a >>> 15, 1 | a);
  t = t + Math.imul(t ^ t >>> 7, 61 | t) ^ t; return ((t ^ t >>> 14) >>> 0) / 4294967296; }; }
const GROUPS = [["small", 0, 100], ["medium", 100, 400], ["large", 400, 1e9]];
function run(R, seed) {
  const rnd = mulberry32(seed), prs = [];
  for (let t = 0; t < 2000; ) {
    t += -Math.log(1 - rnd()) * 2.25; const u = rnd();
    const L = Math.round(u < 0.6 ? 20 + rnd() * 80 : u < 0.9 ? 100 + rnd() * 300 : 400 + rnd() * 800);
    prs.push({ L, open: t, ready: t, rounds: 0, work: 0 });
  }
  const free = Array(R).fill(0), queue = prs.slice();
  while (queue.length) {
    queue.sort((a, b) => a.ready - b.ready);
    const p = queue.shift(), r = free.indexOf(Math.min(...free)), w = 0.2 + p.L / 250;
    free[r] = Math.max(free[r], p.ready) + w; p.work += w; p.rounds++;
    if (rnd() < Math.min(0.7, p.L / 1000)) { p.ready = free[r] + 1; queue.push(p); } else p.done = free[r];
  }
  const busy = prs.reduce((s, p) => s + p.work, 0);
  console.log(`reviewers=${R}  load per reviewer=${Math.round(busy / 20 / R)}%`);
  // time = share of all review hours spent on this group
  for (const [name, lo, hi] of GROUPS) {
    const g = prs.filter(p => p.L >= lo && p.L < hi), d = g.map(p => p.done - p.open).sort((a, b) => a - b);
    const sum = f => g.reduce((s, p) => s + f(p), 0);
    console.log(`  ${name.padEnd(7)}${String(g.length).padStart(4)} PRs  rounds ${(sum(p => p.rounds) / g.length).toFixed(2)}` +
      `  time ${String(Math.round(sum(p => p.work) / busy * 100)).padStart(2)}%` +
      `  median ${d[g.length >> 1].toFixed(1).padStart(4)} h  p90 ${d[Math.floor(g.length * 0.9)].toFixed(1).padStart(5)} h`);
  }
}
run(1, 44); run(2, 44);

reviewers=1  load per reviewer=79%
  small   554 PRs  rounds 1.06  time 16%  median  4.0 h  p90  18.0 h
  medium  252 PRs  rounds 1.36  time 26%  median  7.0 h  p90  24.8 h
  large    82 PRs  rounds 3.06  time 58%  median 16.6 h  p90  86.2 h
reviewers=2  load per reviewer=39%
  small   554 PRs  rounds 1.06  time 17%  median  0.5 h  p90   2.1 h
  medium  252 PRs  rounds 1.32  time 26%  median  1.4 h  p90   5.0 h
  large    82 PRs  rounds 3.04  time 58%  median  7.9 h  p90  30.6 h

সময় সীমিত হলে কোন PR আগে, কোথায় বেশি মনোযোগ? কিছু দল সহজ একটি ঝুঁকি-হিসাব রাখে — কত লাইন, কতগুলো ফাইল, সংবেদনশীল জায়গা ছুঁয়েছে কি না, code বদলালেও test ছোঁয়া হয়েছে কি না। নিচের ওজনগুলো বানানো:

import math
# Toy "diff risk" score to decide where review attention goes first.
# Weights are invented for teaching; a team would tune them on its own history.
SENSITIVE = ("auth/", "payment/", "migrations/")

def risk(files):              # files: list of (path, added, deleted); code = .py
    code = [f for f in files if f[0].endswith(".py") and not f[0].startswith("tests/")]
    tests = [f for f in files if f[0].startswith("tests/")]
    lines = sum(a + d for _, a, d in code)
    score, why = 10 * math.log2(1 + lines / 50), [f"{lines} code lines"]
    score += 3 * len(code); why.append(f"{len(code)} code files")
    hot = sorted({p for p, _, _ in code if p.startswith(SENSITIVE)})
    if hot: score += 15; why.append("sensitive: " + ", ".join(hot))
    if code and not tests: score += 20; why.append("code changed, no test touched")
    return round(score), why

PRS = {
    "#101 fix typo in receipt": [("ui/receipt.html", 1, 1)],
    "#102 add coupon limit":    [("cart/coupon.py", 30, 4), ("tests/test_coupon.py", 40, 0)],
    "#103 refund rounding":     [("payment/refund.py", 12, 6)],
    "#104 new login throttle":  [("auth/login.py", 140, 20), ("auth/limits.py", 60, 0),
                                 ("config/app.toml", 3, 1), ("tests/test_login.py", 90, 0)],
    "#105 rename helpers":      [(f"util/h{i}.py", 8, 8) for i in range(12)],
}
ranked = sorted(((*risk(f), name) for name, f in PRS.items()), reverse=True)
for score, why, name in ranked:
    print(f"{score:3d}  {name}")
    print("      " + "; ".join(why))

 79  #105 rename helpers
      192 code lines; 12 code files; code changed, no test touched
 45  #104 new login throttle
      220 code lines; 2 code files; sensitive: auth/limits.py, auth/login.py
 42  #103 refund rounding
      18 code lines; 1 code files; sensitive: payment/refund.py; code changed, no test touched
 10  #102 add coupon limit
      34 code lines; 1 code files
  0  #101 fix typo in receipt
      0 code lines; 0 code files

৯ · Review-এর বিকল্প আর সঙ্গী — pair programming আর mob programming

PR-ভিত্তিক review “পরে দেখা”: লেখক শেষ করেন, তারপর reviewer পড়েন। আরেকটি পথ হলো কাজের সময়েই একসাথে থাকা।

২০০০-এর আশেপাশে Laurie Williams-সহ কয়েকজনের পরীক্ষা (মূলত শিক্ষার্থীদের ওপর) pair-এ কম ত্রুটি আর কিছুটা বাড়তি মোট সময়ের কথা জানিয়েছিল; পরের meta-analysis-এ ফল মিশ্র। তাই বেশিরভাগ দল মিশিয়ে চলে: রোজকার কাজে PR review, কঠিন কাজে জোড়ায় বসে লেখা। Bus factor — কতজন হঠাৎ না থাকলে project আটকে যায় — বাড়ানোর তিনটিই উপায়।

১০ · AI-এর যুগে — AI reviewer, আর AI-লেখা কোডের review

এখন দুই দিক থেকে AI review-এর কাজে ঢুকেছে: অনেক platform-এ AI নিজেই PR-এ মন্তব্য দেয়, আর অনেক PR-এর কোডের বড় অংশই AI-এর লেখা। দুটিতেই মানুষের review আরও জরুরি — AI-লেখা কোড দেখতে পরিপাটি, তাই চোখ ঢিলে হয়ে যায়।

AI-এর সাহায্যে review — চেকলিস্ট

• AI-এর প্রতিটি blocking দাবি নিজে কোড পড়ে বা চালিয়ে যাচাই করেছি
• AI-লেখা কোড মানুষের লেখা কোডের মতোই, একই চেকলিস্টে review করেছি — পরিপাটি চেহারায় ভুলিনি
• AI-লেখা test-এ একটি ইচ্ছাকৃত ভুল ঢুকিয়ে দেখেছি test fail করে কি না
• AI-এর দেওয়া tool flag বা Git command অফিসিয়াল documentation বা git help-এ মিলিয়েছি
• Prompt-এ কোনো secret, token বা ব্যক্তিগত তথ্য পাঠাইনি
• শেষ approve-এর দায় আমার — দলের নিয়ম অনুযায়ী PR-এ জানিয়েছি কোন অংশে AI-সহায়তা ছিল