Mert Özen Her Satırda Daha İleri
Test ve Kalite Code Review — #03

Reviewer Olarak Neye Bakmalı? Okuma Sırası ve .NET + Angular PR'ı için Pratik Checklist

Mert Özen 14 Kas 2026 14 dk 14 görüntülenme
Reviewer Olarak Neye Bakmalı? Okuma Sırası ve .NET + Angular PR'ı için Pratik Checklist

Reviewer bir PR'ı önce açıklama, sonra tasarım, en son ayrıntı sırasıyla okumalı ve yalnız diff'e değil, değişikliğin etkilediği koda da bakmalı. Testleri yeşil olan bir kupon PR'ında bu sırayla bir N+1 ve module sınırı ihlali, negatif toplama yol açan değişmemiş bir kural, UTC hatası, implementation'ı tekrar eden bir test, eski siparişleri sıfırlayan bir migration, log'da e-posta ve Angular'da stored XSS bulunuyor.

Sürüm notu: Bu yazıdaki kodlar .NET 10 (SDK 10.0.402), Entity Framework Core (EF Core; .NET'in veritabanı erişim kütüphanesi) 10.0.15, SQL Server 2025, xUnit.net v3 4.0.3, Angular 22.2 (TypeScript 6.0) ve Vitest 5.1 ile derlenip denenmiştir.

Önce Tahmin Et

Sitedeki serilerin ortak örnek uygulaması Bookstore'un (bir online kitapçı) kupon özelliği, bir önceki yazıda üç katmanlı bir stack olarak açıldı. Ayşe şimdi en üstteki katmanı, kuponu sepete ve siparişe uygulayan pull request'i (PR) review ediyor: 15 dosya, 332 satır, testler yeşil, açıklama düzgün. Açıklamadaki okuma önerisine uyup CouponService'ten başlıyor:

public sealed class CouponService(BookstoreContext db)
{
    public async Task<Coupon?> FindValidAsync(string code, decimal subtotal, CancellationToken ct)
    {
        string normalized = code.Trim().ToUpperInvariant();
        Coupon? coupon = await db.Coupons.AsNoTracking().FirstOrDefaultAsync(c => c.Code == normalized, ct);
        if (coupon is null || coupon.ExpiresAtUtc < DateTime.Now || subtotal < coupon.MinSubtotal)
        {
            return null;
        }
        return coupon;
    }

    public async Task<decimal> CalculateDiscountAsync(Coupon coupon, CartPrice price, CancellationToken ct)
    {
        decimal eligible = 0m;
        foreach (PricedLine line in price.Lines)
        {
            if (coupon.CategoryId is null)
            {
                eligible += line.LineTotal;
                continue;
            }
            int categoryId = await db.Books.AsNoTracking()
                .Where(b => b.Id == line.Book.Id)
                .Select(b => b.CategoryId)
                .FirstAsync(ct);
            if (categoryId == coupon.CategoryId)
            {
                eligible += line.LineTotal;
            }
        }
        return CouponRules.CalculateDiscount(coupon, eligible);
    }
}

Beş farklı kitap olan bir sepete CLASSIC15 uygulandıktan sonra sepeti getiren tek bir GET /api/carts/{id} isteği veritabanına kaç SQL komutu gönderir? Ve bu 40 satırda, testlerin hiçbirinin yakalamadığı kaç sorun görüyorsun?

Okuma Sırası: Önce Amaç, Sonra Tasarım, En Son Ayrıntı

Bir PR'ı ilk dosyadan son dosyaya doğru okumak en yaygın ve en verimsiz yol. Google'ın Engineering Practices rehberi üç adım öneriyor; Bookstore'da da aynısını yapıyoruz.

Birinci adım büyük resim. Açıklamayı oku ve değişikliğin yapılması gerekip gerekmediğine karar ver. Kuponun nasıl hesaplanacağı Selin'le konuşulmuş, story'si var, kapsam dışında bırakılanlar yazılmış. Bu adımda "bu PR hiç olmamalı" demek gerekiyorsa bunu hemen ve gerekçesiyle söylersin; satır yorumlarına girmezsin.

İkinci adım ana parçalar. En çok mantık değişikliği olan dosyaları bul ve tasarımı orada değerlendir. Burada bir tasarım sorunu görürsen yorumunu hemen gönder, PR'ın geri kalanını okumayı bekletme. Yazar büyük ihtimalle bu PR'ın üstüne yeni iş kuruyor; tasarım itirazı ne kadar geç gelirse o kadar çok kod çöpe gider. Testleri bu adımda okumak da işe yarar: testler değişikliğin ne yapması gerektiğini anlatan en kısa spesifikasyondur.

Üçüncü adım geri kalan dosyalar. Tasarım sağlamsa diğer dosyaları mantıklı bir sırayla, hiçbirini atlamadan okursun. PR bu sırayı izlenemeyecek kadar büyükse, önceki yazıda konuştuğumuz gibi, bölünmesini istemek reviewer'ın hakkı.

Bu sıranın bir tuzağı var: yalnız diff'e bakmak. Diff, değişen satırları gösterir; değişmeyen ama değişiklikten etkilenen kodu göstermez. Ayşe'nin bulduğu sorunlardan biri tam da böyle bir yerde.

Tasarım ve Performans: Module Sınırı ve N+1

Tahminin cevabı sekiz. API'yi EF Core'un komut log'u açık çalıştırıp sepeti bir kez getirdiğimizde log'da sekiz Executed DbCommand satırı var: sepet, kitaplar, kupon ve her satır için ayrı bir kategori sorgusu. Son beşi aynı:

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (1ms) [Parameters=[@line_Book_Id='?' (DbType = Int32)], CommandType='Text', CommandTimeout='30']
      SELECT TOP(1) [b].[CategoryId]
      FROM [catalog].[Books] AS [b]
      WHERE [b].[Id] = @line_Book_Id
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (1ms) [Parameters=[@line_Book_Id='?' (DbType = Int32)], CommandType='Text', CommandTimeout='30']
      SELECT TOP(1) [b].[CategoryId]
      FROM [catalog].[Books] AS [b]
      WHERE [b].[Id] = @line_Book_Id
...

Bu klasik bir N+1: N satır için N ayrı sorgu. Her biri bir milisaniye sürdüğü için yerelde kimse fark etmez; ama aynı method hem sepet görünümünde hem sipariş verirken, yani sipariş transaction'ının içinde de çalışıyor. Sepetteki kitap sayısı ve aynı anda açık sepet sayısı arttıkça sorgu sayısı da çarpılarak artar. Testler bunu yakalamaz, çünkü testler sonucu kontrol eder, sorgu sayısını değil.

N+1'in arkasında daha önemli bir tasarım sorunu var. CatalogService'in başında bir yorum duruyor: diğer module'lerin kataloğu okuduğu tek kapı bu service. CouponService ise Ordering module'ünde ve Catalog'un tablosuna doğrudan gidiyor. Bugün tek bir DbContext var ve bu çalışıyor; ama Catalog bir gün ayrı bir veritabanına ya da servise taşındığında bu satır kırılır. Üstelik ihtiyaç duyulan bilgi zaten elde: sepet fiyatlanırken kitaplar ICatalogService.GetByIdsAsync ile tek sorguda okunuyor. Ayşe'nin yorumu bu iki noktayı birlikte söylüyor ve bir çözüm öneriyor: kategoriyi BookInfo'ya ekle, indirimi elindeki satırlardan hesapla. Emre'nin düzeltmesi:

public sealed record BookInfo(int Id, string Isbn, string Title, decimal Price, int CategoryId);

// Fiyatlanmış satırlar kitabın kategorisini zaten taşıyor; satır başına ek sorgu yok.
public static decimal CalculateDiscount(Coupon coupon, CartPrice price)
{
    decimal eligible = price.Lines
        .Where(l => coupon.CategoryId is null || l.Book.CategoryId == coupon.CategoryId)
        .Sum(l => l.LineTotal);
    return CouponRules.CalculateDiscount(coupon, eligible);
}

Aynı sepet için aynı istek artık üç komut gönderiyor: sepet, kitaplar (kategoriyle birlikte) ve kupon. Method async olmaktan da çıktı; veritabanına dokunmayan bir hesap, saf bir fonksiyon olarak test edilebilir. EF Core'da N+1'in diğer biçimleri ve tracking'in maliyeti EF Core serisinde anlatıldı; review'da bakılacak işaret basit: döngü içinde await ile veritabanına giden bir sorgu.

Davranış: Değişmeyen Ama Etkilenen Kod

Ayşe CouponRules.CalculateDiscount çağrısını görünce diff'te olmayan bir dosyayı açıyor. Bu fonksiyon stack'in ikinci katmanında geldi ve onaylandı; bu PR onu değiştirmiyor ama ona ilk kez gerçek veri gönderiyor:

public static decimal CalculateDiscount(Coupon coupon, decimal eligibleSubtotal) => coupon.Kind switch
{
    CouponKind.Percent => Math.Round(eligibleSubtotal * coupon.Value / 100m, 2, MidpointRounding.AwayFromZero),
    CouponKind.Amount => coupon.Value,
    _ => throw new UnreachableException()
};

Tutar kuponu, uygun tutar ne olursa olsun kuponun değerini döndürüyor. WELCOME25'in minimum sepet tutarı yok; 19,90 liralık bir sepette indirim 25 lira olur ve toplam eksiye düşer. Bir kategori kuponunda sepette o kategoriden hiç kitap yoksa uygun tutar sıfırdır ve indirim yine kuponun değeridir. İkinci katmanın testleri yalnız happy path'i deniyordu. Ayşe tartışmak yerine iki test yazıp çalıştırıyor:

[Fact]
public void Amount_discount_never_exceeds_the_eligible_subtotal()
{
    Coupon welcome25 = new() { Code = "WELCOME25", Kind = CouponKind.Amount, Value = 25m };

    Assert.Equal(19.90m, CouponRules.CalculateDiscount(welcome25, 19.90m));
}

[Fact]
public void Nothing_eligible_means_no_discount()
{
    Coupon welcome25 = new() { Code = "WELCOME25", Kind = CouponKind.Amount, Value = 25m };

    Assert.Equal(0m, CouponRules.CalculateDiscount(welcome25, 0m));
}
failed Bookstore.Api.Tests.CouponRulesTests.Amount_discount_never_exceeds_the_eligible_subtotal (1ms)
  Assert.Equal() Failure: Values differ
  Expected: 19,90
  Actual:   25
failed Bookstore.Api.Tests.CouponRulesTests.Nothing_eligible_means_no_discount (0ms)
  Assert.Equal() Failure: Values differ
  Expected: 0
  Actual:   25
Test run summary: Failed!
  total: 20
  failed: 2
  succeeded: 18

Kırmızı bir test, "bence burada bir sorun var" cümlesinden daha ikna edicidir. Düzeltme tek satır: CouponKind.Amount => Math.Min(coupon.Value, eligibleSubtotal).

Aynı dosyada ikinci bir davranış sorunu, son kullanma kontrolündeki DateTime.Now. Kupon saatleri veritabanında UTC (eşgüdümlü evrensel saat) olarak tutuluyor, DateTime.Now ise makinenin yerel saatini veriyor ve DateTime karşılaştırması Kind'a bakmıyor. Saat dilimi İstanbul olan bir makinede küçük bir denemeyle:

DateTime expiresAtUtc = new(2026, 12, 31, 20, 59, 59, DateTimeKind.Utc);
DateTime now = new DateTimeOffset(2026, 12, 31, 22, 30, 0, TimeSpan.FromHours(3)).LocalDateTime;

Console.WriteLine($"now (local) = {now:yyyy-MM-dd HH:mm} {now.Kind}, as UTC = {now.ToUniversalTime():HH:mm}");
Console.WriteLine($"expired?    = {expiresAtUtc < now}");
Console.WriteLine($"expired?    = {expiresAtUtc < now.ToUniversalTime()} (compared in UTC)");
$ dotnet run expiry.cs
now (local) = 2026-12-31 22:30 Local, as UTC = 19:30
expired?    = True
expired?    = False (compared in UTC)

Yılbaşı kampanyası Türkiye saatiyle gece yarısına kadar sürmeliydi; bu kodla saat 21'den sonra kupon geçersiz görünür. Production sunucusu UTC'de çalışıyorsa sorun ortaya çıkmaz, geliştiricinin makinesinde çıkar ve test edilemez. Uygulama zaten TimeProvider'ı dependency injection (DI) container'ına kayıt ediyor; düzeltme, CouponService'in de onu almak ve clock.GetUtcNow().UtcDateTime ile karşılaştırmak.

Testler Davranışı mı, Implementation'ı mı Doğruluyor?

PR'daki ilk kupon testi şöyleydi:

CartView? cart = await response.Content.ReadFromJsonAsync<CartView>(ct);
Assert.Equal(Math.Round(cart!.Subtotal * 10 / 100m, 2), cart.Discount);
Assert.Equal(cart.Subtotal - cart.Discount, cart.Total);

Test, beklenen değeri implementation'daki formülü tekrar ederek hesaplıyor. Formül yanlışsa test de aynı şekilde yanlış hesaplar ve yeşil kalır. Bu testte bir de gizli fark var: kod MidpointRounding.AwayFromZero kullanıyor, test varsayılan yuvarlamayı. Dune'un fiyatında fark çıkmıyor; Emma'da çıkıyor:

decimal subtotal = 249.90m;
Console.WriteLine(Math.Round(subtotal * 15 / 100m, 2));
Console.WriteLine(Math.Round(subtotal * 15 / 100m, 2, MidpointRounding.AwayFromZero));
$ dotnet run round.cs
37,48
37,49

Ayşe'nin önerisi, beklenen değerleri elle hesaplanmış somut sayılar olarak yazmak ve reddedilen kuponları da test etmek. Testin neyi koruması gerektiğini ve AAA (Arrange-Act-Assert; testi hazırlık, çalıştırma ve doğrulama adımlarına ayıran düzen) yapısını Unit Test serisinin "İyi Bir Unit Test'in Anatomisi: AAA, İsimlendirme, Tek Davranış ve Okunabilir Assertion'lar" yazısında konuştuk. Emre'nin yeni testlerinden biri:

[Fact]
public async Task CLASSIC15_discounts_only_classics()
{
    Guid cartId = await CreateCartWithAsync(Dune, Emma);

    CartView cart = await ApplyAsync(cartId, "classic15");

    // 249,90'ın %15'i = 37,485; sıfırdan uzağa yuvarlanır
    Assert.Equal("CLASSIC15", cart.CouponCode);
    Assert.Equal(37.49m, cart.Discount);
    Assert.Equal(701.41m, cart.Total);
}

Bu test tek başına üç şeyi koruyor: kodun büyük-küçük harfe duyarsız eşleşmesini, kategori filtresini ve yuvarlama kuralını. Yanına SAVE50'nin minimum tutarın altında reddedildiğini ve bilinmeyen bir kodun 400 döndüğünü kontrol eden testler eklendi.

Migration: Mevcut Veri ve Eski Sürüm

Migration'ın designer dosyası okunmaz ama Up method'u mutlaka okunur. Ayşe'nin en kolay yolu migration'ın üreteceği SQL'e bakmak:

$ dotnet ef migrations script AddCoupons ApplyCoupons --project src/Bookstore.Api
BEGIN TRANSACTION;
ALTER TABLE [ordering].[Orders] ADD [CouponCode] varchar(32) NULL;

ALTER TABLE [ordering].[Orders] ADD [Discount] decimal(12,2) NOT NULL DEFAULT 0.0;

ALTER TABLE [ordering].[Orders] ADD [Subtotal] decimal(12,2) NOT NULL DEFAULT 0.0;

ALTER TABLE [ordering].[Carts] ADD [CouponCode] varchar(32) NULL;
...

Subtotal yeni, zorunlu bir kolon ve varsayılan değeri sıfır. Kupon öncesi verilmiş bir sipariş olan veritabanında migration'ı uygulayıp siparişe bakınca:

Id Subtotal Discount Total CouponCode
-- -------- -------- ----- ----------
1 .00 .00 489.00 NULL

Eski siparişlerin ara toplamı sıfır görünüyor. Bu alanı okuyan bir rapor ya da sipariş geçmişi ekranı yanlış bilgi gösterir. Düzeltme migration'a bir backfill eklemek:

// Kuponlardan önceki siparişlerde indirim yoktu: ara toplam, toplama eşittir.
migrationBuilder.Sql("UPDATE [ordering].[Orders] SET [Subtotal] = [Total] WHERE [Subtotal] = 0;");
Id Subtotal Discount Total
-- -------- -------- -----
1 489.00 .00 489.00

Backfill mevcut satırları düzeltiyor ama bir soru daha var: deploy sırasında eski sürüm bir süre daha çalışırsa ne olur? Eski kod Subtotal'ı bilmez; migration'dan sonra verdiği siparişler yine sıfırla kaydedilir. Ayşe bunu bloklayıcı saymıyor, çünkü Bookstore'un deploy'unda iki sürüm aynı anda sipariş almıyor; ama PR açıklamasının "Risk and rollback" bölümüne yazılmasını istiyor. Eski ve yeni sürümün aynı anda çalıştığı ortamlarda doğru yol expand-contract: önce nullable kolon, sonra iki sürümün de yazdığı kod, en son zorunlu kılmak. Ayrıntısı Deployment Stratejileri serisinin "Migration'ların Deploy'daki Yeri: Expand-Contract, EF Core Migration Bundle ve Geriye Uyumlu Şema" yazısında.

Güvenlik ve Kişisel Veri: Log, XSS ve Yetki

Sipariş verildiğinde yazılan log satırı API'nin konsolunda şöyle görünüyor:

info: Bookstore.Ordering.OrderService[0]
      Order 2 placed by ada@example.com with coupon CLASSIC15, discount 136.32

E-posta adresi kişisel veri. Log'lar çoğu zaman uygulama veritabanından daha geniş bir kitleye açıktır, daha uzun saklanır ve silme taleplerinin kapsamına girmesi zordur. Log satırının işi siparişi bulmayı sağlamak; bunun için müşteri numarası yeterli. Düzeltmeden sonra aynı satır:

info: Bookstore.Ordering.OrderService[0]
      Order 2 placed by customer 2 with coupon BOOK10, discount 24.99

Angular tarafında kupon açıklaması pazarlama ekibinin back office'te yazdığı HTML içeriyor (<strong>15%</strong> off classics). Emre bunu göstermek için Angular'ın sanitizer'ını devre dışı bırakmış:

private readonly sanitizer = inject(DomSanitizer);
protected readonly couponDescription = computed(() =>
  this.sanitizer.bypassSecurityTrustHtml(this.store.cart()?.couponDescription ?? ''));

bypassSecurityTrustHtml, review'da gördüğünde her zaman durman gereken bir isim. Açıklama veritabanından geliyor; back office hesabı ele geçirilirse ya da bir gün açıklamayı başka bir sistem yazarsa, her müşterinin sepet sayfasında script çalıştıran bir stored XSS (cross-site scripting; sayfaya zararlı script enjekte edilmesi) açılır. Ayşe yine testle gösteriyor:

it('keeps the formatting but drops script-capable markup', async () => {
  // ...
  TestBed.inject(HttpTestingController).expectOne('/api/carts/cart-1').flush(cartWith(
    '<strong>15%</strong> off classics<img src="x" onerror="alert(document.cookie)">'));
  // ...
  expect(description.querySelector('strong')?.textContent).toBe('15%');
  expect(description.querySelector('img[onerror]')).toBeNull();
});
 FAIL   bookstore-web  src/app/cart/cart-page.spec.ts > CartPage coupon description > keeps the formatting but drops script-capable markup
AssertionError: expected <img src="x" …(1)></img> to be null

- Expected:
null

+ Received:
<img
  onerror="alert(document.cookie)"
  src="x"
/>

Düzeltme kod silmek: DomSanitizer ve computed kalkıyor, template doğrudan [innerHTML]="store.cart()!.couponDescription" kullanıyor. Angular bu durumda değeri kendi sanitizer'ından geçirir; <strong> kalır, onerror düşer ve test geçer. Angular'daki DOM (tarayıcının sayfayı temsil ettiği nesne ağacı) XSS sink'lerinin tam listesi ve review'da bakılacaklar Frontend Güvenliği serisinin "Angular'da DOM XSS Sink Envanteri: ElementRef, Renderer2, Dinamik URL'ler, HTML Üreten Kütüphaneler ve Code Review Listesi" yazısında.

Yetki tarafında Ayşe'nin bir sorusu var, bir bulgusu değil: kupon endpoint'leri kimlik doğrulama (authentication) istemiyor. Bookstore'da sepet anonim ve kimliği tahmin edilemeyen bir GUID (dünya genelinde benzersiz kimlik değeri); mevcut sepet endpoint'leri de aynı modelle çalışıyor. Kupon bu modeli değiştirmediği için soru PR'da cevaplanıp kapatılıyor. Input validation ise iyi yapılmış: ApplyCouponRequest'te [Required, StringLength(32)] var ve veritabanındaki kolonla aynı uzunlukta. Ayşe bunu da yorum olarak yazıyor; iyi olanı söylemek, bir sonraki PR'da aynı özenin tekrarlanmasını sağlar.

Bookstore'un Review Checklist'i

Ayşe'nin bulduklarının hiçbiri tesadüf değil; hepsi ekibin kısa checklist'inden geliyor. Bu liste bir form değil, okurken akılda tutulan sorular:

  • Davranış: açıklamadaki iddiayı kod yapıyor mu? Sıfır, negatif, boş liste, sınır değer, süresi dolmuş kayıt gibi edge case'ler ne oluyor? Değişmeyen ama yeni veriyle çağrılan kod var mı?
  • Testler: beklenen değerler somut sayı mı, yoksa implementation'ın tekrarı mı? Hata ve ret yolları test ediliyor mu?
  • EF Core: döngü içinde sorgu (N+1), gereksiz tracking, başka module'ün tablosuna doğrudan erişim.
  • Migration: mevcut satırlara ne olur, eski sürüm migration'dan sonra çalışmaya devam edebilir mi, Down gerçekten geri alıyor mu?
  • Güvenlik: yetki kontrolü, input validation, log'da kişisel veri, zaman ve para hesaplarında UTC ve yuvarlama.
  • Angular: bypassSecurityTrust*, innerHTML ve ElementRef.nativeElement kullanımı; component'te takeUntilDestroyed olmadan açılan subscribe'lar; template'te method çağırmak yerine computed signal.

Listede olmayanlar da önemli: girinti, boşluk, var kullanımı, naming biçimi, kullanılmayan using. Bunlar insanın değil aracın işi. Bookstore'da Directory.Build.props'taki TreatWarningsAsErrors analyzer uyarılarını build hatasına çeviriyor; formatter ve analyzer'ların CI kapısına nasıl bağlandığı Coding Conventions ve Ekip Standartları serisinin "Kuralları Otomatikleştirmek: Pre-commit Hook, CI Kapısı, Legacy Codebase'e Kademeli Geçiş ve .git-blame-ignore-revs" yazısında. Analyzer kurallarının daha derin kısmı Statik Analiz serisinin konusu. Checklist'i ikinci kez aynı yorumu yazdığında güncelle: bir kural iki PR'da yorum olarak tekrarlandıysa ya listeye ya da bir araca taşınmalı.

Review'un sonunda Emre beş commit ekliyor: indirim sınırı, kategori ve TimeProvider, migration backfill'i, somut testler ve Angular sanitizer'ı. Son durum:

$ dotnet test
Test run summary: Passed!
  total: 23
  failed: 0
  succeeded: 23
  skipped: 0
  duration: 2s 211ms
$ cd web/bookstore-web && npm test -- --watch=false
 Test Files  2 passed (2)
      Tests  2 passed (2)

Üretimde Dikkat

Review'da yakalanan her sorun bir soruyu da beraberinde getirir: bunu bir dahaki sefere review'dan önce ne yakalardı? N+1 için integration testlerinde sorgu sayısını sayan bir kontrol ya da EF Core log'larında bir istekteki komut sayısını izlemek; DateTime.Now için yasaklı API listesi tutan bir analyzer; log'daki kişisel veri için structured logging'de alan adına bakan bir kural. Review'un işi her seferinde aynı hatayı bulmak değil, tekrar eden hatayı bir araca devretmektir.

Reviewer'ın zamanı da bir kaynak. Bu PR'ın review'u bir saate yakın sürdü, çünkü Ayşe testleri kendisi yazıp çalıştırdı ve migration'ı gerçek bir veritabanında denedi. Her PR için bu yapılmaz; ama migration, para hesabı ve güvenlik içeren PR'larda yapılmalı. Hangi PR'ın bu kadar dikkat istediğini PR'ın label'ları ve CODEOWNERS söyleyebilir; beşinci yazıda bunlara bakacağız.

Sık Düşülen Tuzaklar

Yalnız diff'e bakmak. CouponRules bu PR'da değişmedi ama hatası bu PR'la görünür oldu. Değişen bir satırın çağırdığı ya da onu çağıran kodu açmak, review'un en çok atlanan adımı.

Checklist'i mekanik uygulamak. "Authorization: var mı?" kutusunu işaretlemek kolay; asıl soru bu endpoint'in yetki modelinin geri kalan sistemle tutarlı olup olmadığı. Liste düşünmeyi tetiklemek için var, yerine geçmek için değil.

Testlere güvenip testleri okumamak. CI yeşil diye testlerin doğru şeyi doğruladığını varsaymak, implementation'ı tekrar eden bir testin bütün hataları gizlemesine izin verir.

Her şeyi aynı ağırlıkta yazmak. N+1, stored XSS ve bir isim önerisi aynı tonda yazılırsa yazar hangisinin merge'i engellediğini bilemez. Yorumların hangisinin bloklayıcı olduğunu açıkça söylemenin yollarını bir sonraki yazıda konuşacağız.

Tasarım yorumunu sona saklamak. Ayşe module sınırı yorumunu ilk okumada gönderdi; Emre diğer yorumlar gelirken düzeltmeye başlayabildi.

Küçük Bir Deneme

★☆☆ Kendi projende EF Core komut log'unu açıp (Microsoft.EntityFrameworkCore.Database.Command kategorisini Information seviyesine çek) en sık kullanılan endpoint'i bir kez çağır. Kaç SQL komutu görüyorsun ve hangileri döngü içinden geliyor?

★★☆ Bookstore'da FindValidAsync artık TimeProvider alıyor. Süresi dolmuş bir kuponun reddedildiğini, saati ileri alınabilen bir TimeProvider ile test eden bir integration testi yaz. WebApplicationFactory'de hangi service'i değiştirmen gerekiyor?

★★★ Ekibinin son on PR'ındaki review yorumlarını bu yazıdaki checklist'in başlıklarına dağıt. Hiç yorum almayan başlık var mı? Aynı yorumun iki kez yazıldığı yerleri bul ve her biri için onu yakalayacak bir analyzer, test ya da CI kontrolü öner.

Ayşe neye bakacağını biliyordu ve buldu. Ama bulduğunu nasıl yazdığı da en az bulduğu kadar önemli: aynı N+1 yorumu Emre'yi ya düşündürür ya da savunmaya geçirir. Bir sonraki yazıda review'un iletişim tarafına geçiyoruz: yorumun dili, Conventional Comments, anlaşmazlıkta izlenecek yol ve review yorgunluğu.