Problem: heç kimin xoşlamadığı review-lar

1 800 dəyişdirilmiş sətri olan Pull Request üç gün gözləyir. Reviewer sürətlə göz gəzdirir, "LGTM" yazır və təsdiqləyir. Bir həftə sonra reviewer-in formal olaraq təsdiqlədiyi kodda bug çıxır. Başqa komandada əksinə: hər Pull Request-ə adlandırma və mötərizə üslubu haqqında otuz şərh yazılır, müəllif özünü hədəf kimi hiss edir, növbəti PR daha böyük olur, çünki "hər dəfə bunu yaşamaq istəmirəm, dəyişiklikləri yığıb göndərəcəyəm". Hər iki komandada review var. Heç birində review mədəniyyəti yoxdur.

Review nə üçündür

Review bir neçə məqsədli söhbətdir, təxminən dəyərə görə sıra ilə:

  1. Düzgünlük və risk. Kod dediyini edirmi? Sərhəd halları, xətalar və təhlükəsizlik nəzərə alınıbmı?
  2. Dizayna uyğunluq. Arxitekturaya uyğundurmu, növbəti feature üçün problem yaratmırmı?
  3. Ortaq bilik. Ən azı bir nəfər də bu kodu indi başa düşür.
  4. Oxunaqlılıq və ardıcıllıq. Yeni gələn bunu dəstəkləyə bilərmi?

Format xətalarını tapmaq review-un işi deyil; bunu CI-dəki formatter və linter mübahisəsiz edir. Review həm də senior-ların daha ağıllı olduğunu sübut etdiyi qapı deyil. Məqsəd daha yaxşı proqram çatdırmaq və anlayışı yaymaqdır.

Kiçik Pull Request-lər əsasdır

Ölçü artdıqca review keyfiyyəti kəskin düşür, çünki reviewer bütün dəyişikliyi başında saxlaya bilmir. Bir oturuşda başa düşülə bilən və bir iş görən Pull Request hədəfləyin. Praktik üsullar:

  • Refactoring-i davranış dəyişikliyindən ayırın. Birinci PR-da adı dəyişin və ya kodu köçürün, növbətidə məntiqi dəyişin.
  • Natamam işin təhlükəsiz merge olması üçün feature flag və ya əlavə edən dəyişikliklər (əvvəl yeni sütun, sonra onu işlədən kod) istifadə edin.
  • Təsvir yazın: nə, niyə, necə test etmək və nədən əmin deyilsiniz.
  • Əvvəlcə öz diff-inizə baxın. Şərhlərin çoxu iki dəqiqəyə özünüzün tuta biləcəyiniz şeylərdir.

Şərh növləri: nə demək istədiyinizi bildirin

Müəlliflər şərhin məcburi olub-olmadığını təxmin etməyə enerji itirir. Sadə prefikslər razılaşdırın:

PrefiksMənasıMerge-i bloklayır?
blocking:Bug, təhlükəsizlik problemi, pozulmuş müqaviləBəli
question:Başa düşmədim; izah edin və ya kodda aydınlaşdırınCavabdan asılıdır
suggestion:Daha yaxşı fikir, qərar müəllifindirXeyr
nit:Kiçik zövq məsələsi, rahat nəzərə almaya bilərsinizXeyr
praise:Bu yaxşıdır, davam edinXeyr

Tərif bəzək deyil. O, insanlara hansı nümunələri təkrarlamaq lazım olduğunu göstərir.

Ton: sərtdən faydalıya

Eyni tapıntı etibar yarada da bilər, zədələyə də. Müqayisə edin:

Sərt:
  Bu səhvdir. Niyə burada Exception tutursan?

Faydalı:
  blocking: catch (Exception e) InterruptedException və proqramlaşdırma
  xətalarını da udacaq, çağıran isə xətanın izi olmadan boş nəticə
  görəcək. PaymentGatewayException-u tutub order id ilə log yaza
  və domain exception atsaq necə olar?
  Nümunə: OrderService.cancel().

Faydalı variant növü göstərir, nəticəni izah edir, istiqamət təklif edir və nümunəyə yönəldir. Şəxsə yox, koda aiddir ("sən" əvəzinə "bu metod"). Reviewer konteksti bilməyə bilərsə, sual verir. Ortaq məsuliyyət üçün "biz" işlədin, mətndə yaşamayan istehzadan çəkinin.

Müəllifin məsuliyyəti

Müəllif dəyişikliyə merge olunana və işləyənə qədər sahibdir. Bu o deməkdir:

  • PR-ı kiçik, təsvirini aydın saxlamaq.
  • Review istəməzdən əvvəl testləri və formatter-i işə salmaq.
  • Hər şərhə cavab vermək, hətta "düzəldildi" və ya "razı deyiləm, çünki..." ilə.
  • Şərhləri şəxsi qəbul etməmək; onlar dəyişikliyə aiddir.
  • Reviewer-lərə nəyin dəyişdiyini demədən review tarixçəsinin üstündən force-push etməmək.

Qısa reviewer siyahısı

Bürokratiya yox, xatırlatma kimi istifadə edin.

  • Kod təsvirdə yazılanı edirmi?
  • Xəta yolları (timeout, null, qismən uğursuzluq) nəzərə alınıb?
  • Giriş məlumatları yoxlanılır və giriş hüquqları təyin olunur?
  • Bu davranış pozulsa, uğursuz olacaq test varmı? Backend üçün test strategiyasına baxın.
  • Adlar, log və xəta mesajları növbətçi mühəndis üçün faydalıdır?
  • Database migration-lar və rollback təhlükəsizdir?
  • Şərh və ya qərar qeydi tələb edən gözlənilməz bir şey varmı?

Fikir ayrılığı zamanı

Fikir ayrılığı normaldır. Ağlabatan yol:

  1. Qarşı tərəfin fikrini yenidən ifadə edib düzgün başa düşdüyünüzü təsdiqləyin.
  2. Öz məntiqinizi seçim yox, konkret risk və ya fayda ilə izah edin.
  3. İki şərh turundan sonra yaxınlaşma yoxdursa, qısa zəngə və ya söhbətə keçin; yazışma tez gərginləşir.
  4. Zövq məsələsidirsə, komandanın mövcud qaydasına əməl edin və ya qərarı müəllifə buraxın.
  5. Əhəmiyyətli dizayn seçimidirsə, nəticəni yazın, məsələn ADR kimi (ADR yazmaq).

Əvvəlcədən son sözün kimdə olduğunu razılaşdırın. Əksər komandalarda lokal seçimlərdə müəllif, bütün sistemə aid seçimlərdə tech lead qərar verir.

Komandada yeni gələn mühəndis üçün review həm də ən sürətli öyrənmə yoludur. Ona başqalarının Pull Request-lərini oxumağı da tapşırın: bu, kod bazasını, qərarların səbəblərini və komandanın gözləntilərini sənədlərdən daha canlı şəkildə göstərir. Reviewer kimi sual verməkdən çəkinməyin; "bunu niyə belə etdiniz?" sualı tənqid deyil, öyrənmə və aydınlaşdırma vasitəsidir.

Zamanla bu cür suallar hansı qərarların qısa yazılı qeydə layiq olduğunu da üzə çıxarır. Əgər eyni sual müxtəlif PR-larda təkrarlanırsa, bu, komandanın qaydasının yazılmadığını göstərir və onu qısa komanda sənədinə və ya ADR-ə çevirmək vaxtıdır.

Tez-tez edilən səhvlər

  • Yalnız üslub üçün baxıb məntiqi və xəta hallarını qaçırmaq.
  • Müəllif senior olduğu və ya deadline yaxın olduğu üçün oxumadan təsdiqləmək.
  • Pull Request-ləri gözlətmək; yavaş review hər mühəndis üçün gizli vergidir.
  • Hər şeyə bir nəfərin baxması: bu, darboğaz və biliyin tək nöqtədə toplanması deməkdir.
  • Müəllifin həllini öz üslubunuzda yenidən yazmaq, onu mahiyyətinə görə qiymətləndirmək əvəzinə.
  • Review-nu əvvəl aparılmalı olan arxitektura mübahisələrini həll etmək üçün işlətmək.

Kompromislər

Sürətli cavab və hərtərəfli yoxlama əks istiqamətə çəkir. Praktik variant: komandanın razılaşdırdığı cavab vaxtı hədəfi, riskli sahələrdə (ödəniş, autentifikasiya, migration) daha ciddi, az riskli dəyişikliklərdə yüngül baxış. Pair programming bəzi review-ları əvəz edə bilər və dərhal rəy verir, amma yazılı iz qoymur. Avtomatik yoxlamalar küyü azaldır, amma yalançı arxayınlıq yarada bilər; dizayn haqqında yenə insan düşünməlidir.

Komanda üçün siyahı

  • Maksimal PR ölçüsü və ya əhatəsi razılaşdırılıb.
  • Şərh prefiksləri sənədləşdirilib.
  • Formatlama və linting avtomatlaşdırılıb.
  • Gözlənilən review cavab vaxtı məlumdur.
  • Reviewer-lər növbələşir.
  • Riskli sahələrə əlavə diqqət ayrılır.

Növbəti addımlar