Kod incelemesinin başlıca amacı, bakımı zor kodu bulmaktır

• Kod incelemesi, hataları yakalayan ya da bütünlüğü garanti eden bir prosedürden çok, ileride bakımı zor olacak kodu önceden ortaya çıkaran bir süreçtir
• İnceleyen kişi kodu okuduğunda anlamakta zorlanıyorsa, gelecekte bakımını yapacak kişilerin de aynı sorunu yaşama olasılığı yüksektir
• Düzeltmeleri, özgün yazar bağlamı hâlâ hatırlıyorken yapmak daha iyidir; yalnızca kod inceleyerek hataları güvenilir biçimde bulmayı beklemek gerçekçi değildir
• İnceleyen kişi için “hata bul” yerine “anlamaya çalış ve anlamadığın yerleri işaretle” daha uygulanabilir bir görevdir
• İyi bir inceleme, her şeyi kusursuzca kanıtlamak değil; anlaşılmayan noktalara not bırakıp iyileştirme istemekle başlar

---

Kod incelemesinin odağını değiştirmek

• Kod incelemesinin temel amacı, inceleyen kişinin hata bulması değildir; kodda hata olmadığını garanti etmek de değildir
• Yalnızca koda göz atarak genel olarak hata bulunabileceği beklentisi pratikte zayıftır
• Bu yüzden incelemenin odağı “kod doğru mu?”dan çok “başka biri bunu ileride okuyup düzeltebilir mi?” sorusuna yerleşir

Anlaşılması zor kod, bakım için risk sinyalidir

• İnceleyen kişi, kodun ne yaptığını ve nasıl çalıştığını anlamaya çalışarak okur
• Anlaşılmayan kısımlar, gelecekte bakım yapacak kişinin takılma olasılığı olan risk sinyalleri hâline gelir
• Böyle kodları, özgün yazar bağlamı hâlâ hatırlıyorken hemen düzeltmek daha iyidir

Hata bulmaktan daha uygulanabilir bir inceleme görevi

• “Bu kodda hata bulmaya çalış” isteği, başarısını değerlendirmesi zor bir iştir
• Birkaç hata bulunsa bile daha fazla gizli hatayı kaçırmış olabilirsiniz; bu yüzden inceleyen kişi açısından genellikle yalnızca başarısızlık netleşir
• Buna karşılık “bu kodu anlamaya çalış, anlayamıyorsan belirt” isteğinin ölçütü daha nettir
  • Her şeyi kusursuzca anlamak gerekmez
  • Anlamadığınız kısımları kaydetmeniz yeterlidir
  • Bütünü anlamaya çalışıp takıldığınız noktalara not bırakırsanız, incelemenin rolünü yerine getirmiş olursunuz

Pratikte inceleme ölçütü

• İnceleyen kişinin anlamadığı kod, tek başına düzeltilmesi gereken bir şey olabilir
• İnceleme yorumları yalnızca hata bildirimi değil; açıklama eksikliğini, yapı sorunlarını ve okunması zor akışı ortaya çıkarma işlevi de görür
• Bu ölçüte göre önemli olan, kodun doğruluğunu kanıtlamaktan çok, onu daha sonra ekip arkadaşlarının okuyup üzerinde çalışabileceği bir duruma getirmektir

Hata bulmak yan etkidir

• Bu, kod incelemesinin hiç hata bulamayacağı anlamına gelmez
• Hatalar inceleme sırasında bulunabilir; ancak bunu tüm hataları ya da hataların çoğunu bulma yöntemi olarak beklemek zordur
• Daha gerçekçi başarı koşulu, anlaşılabilirliği kontrol etmek ve bakımı zor kısımları özgün yazarla birlikte hemen iyileştirmektir

[GN⁺]

Hacker News yorumları

• Kod incelemesinin tek bir amacı yok. Bakımı zor kodu bulmak önemli, ama tek amaç bu değil; hatta belki de en önemli amaç da değil.
  Geliştirici ya da yapay zeka tuhaf bir yöne gitse bile kötü niyetli kodun birleştirilmesini zorlaştıran bir güvenlik mekanizması olur; soruna çok yakın olmayan biri daha iyi bir yol ya da gözden kaçmış bir problemi görebilir.
  Sistemin başka bölümlerini daha iyi bilen biri etkileşim sorunlarını yakalayabilir; en az bir kişinin o koda aşina olmasını sağlar ve hem yazan hem de inceleyen için öğrenme fırsatı yaratır.
  Özellikle kıdem farkı olduğunda önemlidir: Yeni çalışanlara mentorluk ederken nasıl çalıştığımı görmeleri için onları tüm PR’larıma reviewer olarak ekliyorum; onların PR’larını da inceleyip yönlerini bulmalarına yardımcı oluyorum. Bazen ben de onlardan öğreniyorum.
  Hata yakalamak da elbette var, ama ana mekanizma bu olmamalı; otomatik testlerle yakalanması zor güvenlik ve performans hataları için özellikle önemli.

  • Eski bir gerekçe daha: İnsanlar kodlarının inceleneceğini ve o kod üzerinden gönderen kişinin yetkinliği ve organizasyona uyumu hakkında uzun vadeli bir izlenim oluşacağını bildiklerinde kodu farklı yazarlar.
  • 3. maddeye benzer şekilde, ilgili alt alana daha aşina biri mevcut kodla uyuşmayan ya da sorun çıkarabilecek kısımları yakalayabilir.
  • “Tek amaç” ifadesini, kodu anlayın ve anlaşılmayan kısımlarda sorun çıkarın anlamında okudum. Anladıktan sonra yanlış, aptalca ya da güvensiz kısımları işaret edebilirsiniz deniyorsa bunu makul bulurum.
    Bu özellikle modülerleştirme ve parçalama için geçerli; devasa bir PR’ın tamamını anladıktan sonra kafanızda bir model oluşur ve bakımının mümkün olup olmayacağını, bir gün tam bir kâbusa dönüşüp dönüşmeyeceğini ya da arada bir yerde mi olduğunu görmeye başlarsınız.

• Kod incelemesinin belki de en önemli kısmının bilgi aktarımı olduğunu düşünüyorum.
  Küçük ekibimizde acil bir durum yoksa birleştirmeden önce tüm ekip PR’a onay işareti koyuyor; bu sayede ekipteki herkes mevcut kod tabanının durumunu kabaca biliyor. Daha önce daha silo’lu şirketlerde yaşadığım “bağımlı olduğum tüm sistem ortadan kayboldu” türü sürprizlerden kaçınabiliyoruz.
  Ayrıca nasıl çalıştığına dair soru sormak ve anlayışı geliştirmek için bir alan oluyor. İyi işleyen bir ekipte tüm geliştiricilerin, doğrudan dokunmadıkları kısımlar da dahil olmak üzere, tüm sistemi bir ölçüde anlaması gerekir.
  Bir diğer önemli işlevi de kurumsal bilgiyi kontrol etmek. Yakın zamanda bir tabloyu biraz değiştirdim; bir iş arkadaşım, hesaba katmadığım bir mikroservisin o tabloya yazdığını ve bu yüzden bozulacağını söyledi. O mikroservisin var olduğunu da, o tabloya eriştiğini de bilmiyordum; ama bu kontrol sayesinde daha büyük bir sorunun ve veri temizleme durumunun önüne geçebildik.

  • “Küçük ekibin tamamının birleştirme öncesi PR’a onay vermesi” yavaş hareket eden küçük bir kod tabanında iyi görünüyor; ama büyük ekiplere ya da hızlı ilerleyen kod tabanlarına zorla uygulanırsa kodu üstünkörü tarayıp onay düğmesine basılan biçimsel bir ritüele dönüşmesi kolay.
    Sonunda herkes kendi işiyle meşgul oluyor ve önemli bir PR’ı engelleyen kişi olmak istemediği için sadece onaya basıyor; gerçekte ise kimse kodu incelememiş oluyor.
  • Ekibin kaç kişi olduğunu merak ediyorum. Muhtemelen beş mühendisi geçince böyle bir yöntem ölçeklenmez.
    “Bağımlı olduğum tüm sistem ortadan kayboldu” gibi sorunlar için, o sisteme bağımlı kişi orada olmasa bile yakalayabilecek otomatik testlerin çok önemli olduğunu düşünüyorum.
    Her şey üzerinde ortak sahipliği de güçlü biçimde destekliyorum. İnsanların kod tabanının belirli bölümlerinin, özellikle de kendilerinin yaptığı bileşenlerin fiilen sahibi hâline gelmesi doğal; ama bu silo’lara ve düşük bus factor’a yol açıyor. Bir kişinin bir sisteme sahip olduğu, o sistemin de başka tek bir bileşene bağımlı olduğu bir yapı olmamalı.
  • İş arkadaşın incelemede olmasaydı, o bozucu değişikliğin production’a çıkmasını neyin engelleyeceğini merak ediyorum. Kod incelemesi önemliyse testler bunun neresinde duruyor?
  • Zaten hemen birleştireceğim kodlar için bile bazen PR açıp başka geliştiricileri etiketliyorum. Amaç, neyin neden birleştirildiğini görmeyi kolaylaştıran bir yol sunarak insanların kod tabanı içindeki şeyleri kaçırmamasını sağlamak.
  • Önemsiz olmayan değişikliklerde ya da temizlik işlerinde ikinci bir çift göz her zaman iyidir. Ama “herkes her şeyi okur” yaklaşımı büyük N’e ölçeklenemez.
    Okunacak şey çok fazla olduğunda kimse takip edemez; bu yüzden delege eder, dokümantasyon yazar ve genel bakış oturumları düzenlersiniz.

• Kod incelemesini, yazanın sahip olduğu kodun ekibin ya da projenin sahipliğine geçtiği kapı olarak görmenin en doğrusu olduğunu hep düşünmüşümdür.
  İncelediğim kod artık senin kodun değil, yakında bizim kodumuz olacak koddur. Doğal olarak bakım yapılabilirlik bunun içinde büyük bir faktördür.

  • Gerçekten kıskanılacak, lüks bir ortam.
    Bizim ekip yapay zeka kullanmaya başladı, ben de daha basit bir yönteme geçtim. Yorum bırakmıyorum; sadece “bu çılgınlık düzeyinde mi, yoksa geçebilir mi” diye ikili bir onay kararı veriyorum.
    Zamanımı ve ruh sağlığımı koruyorum.

• Böyle yapınca yalnızca inceleyen ve yazan kişi daha tembel hale gelir
  Kod incelemenin amacı çok yönlüdür. Bakımı zor mu, hata barındırabilir mi, daha basit ve temiz yapılabilir mi, projenin kod stiline uyuyor mu, başkalarının da kodu anlamasını sağlıyor mu, junior ekip üyelerini onboarding’e yardımcı oluyor mu, tasarım kararlarını sanity check’ten geçiriyor mu; bunların hepsi buna dahildir
  Bu tür hafif yazılar genelde tembel kod inceleyicisinin kendini haklı çıkarmasına daha yakındır

  • İncelemede bakılması gereken ayrı bir checklist vardır
    İşlevsel olarak issue ya da PR açıklamasındaki hedefe ulaşıyor mu, geride kalmış debug çıktıları veya gizli API anahtarları gibi gereksiz kodlar var mı, bellek sızıntıları, ele alınmamış edge case’ler, güvenlik açıkları, eski API çağrıları gibi bariz kusurlar var mı diye bakılmalı
    Daha anlaşılır hale getirilebilir mi, abstraction eklemek ya da kaldırmak gerekir mi, değişken ve metot adları daha iyi olabilir mi, fonksiyonel stilin daha fazla mı yoksa daha az mı kullanılması daha iyi olur, bunlara da bakılmalı
    Codebase veya style guide ile tutarlı mı, liste yerine hash set kullanmak ya da lazy evaluation uygulamak gibi bariz performans iyileştirmeleri var mı, testler yeterli mi; bunlar da kontrol edilmeli
    Anlaşılamayan kodun geçmemesi gerektiği iddiasına da tamamen katılmıyorum. Bazı kodları anlamak gerçekten zordur; hedef, işlevsel olarak doğru olurken mümkün olduğunca anlaşılır hale getirmektir
  • Sektör, “yazarı suçlamaktan” “süreci ve ekibi suçlamaya” geçmek için epey çaba harcadı
    Ama bu yazı daha çok yem gibi; “İnsanlar akşam yemeğinin yemek yemek olduğunu sanıyor, ama aslında mesele yemek değil, aile ve arkadaşlarla bağ kurmaktır” demeye benziyor. HN’de iyi işleyen belirli bir tür gevşek indirgemeci mantık
  • Yapay zeka nedeniyle kod yazma ve dağıtma hızı arttığı için vurgu artık incelemeye kaymalı. Kod gerçekten doğru çalışıyor mu, tüm varsayımlarımız doğru mu, istenmeyen yan etkiler yok mu, bunları doğrulamalıyız
    İnceleme ve debugging’in, kod yazma ve üretmeden çok daha fazla zaman aldığını hissettim; sadece “çalışmasını ummak” ise asla iyi bitmez

• “Koda bakarak genelde hata bulunamaz” sözü hiç doğru değil. Her soyutlama seviyesinde yeterince bulunabilir ve bunlara code smell denir
  Kapatılmamış file descriptor’lar, await edilmemiş coroutine’ler, hatayı kaydetmeden bir değere geri dönen dev try/catch blokları, yanlış type casting vb. bunların hepsine dahildir
  Genel ilke olarak, type checker’ı, compiler’ı ve runtime’ı yalnızca geçilmesi gereken aşamalar olarak görmemek gerekir. Bu aşamalarla birlikte çalışmalı, bunların değerli araçlar olduğunu kabul etmeli ve onlara karşı çalışmamalıyız

  • Ne demek istediğini bilmiyorum. Kodu çalıştırmadan yalnızca kod incelemesiyle hata yakaladığım oldu; tersine, benim kodumda başkasının bunu yaptığı da oldu, başkalarının incelemelerinde de böyle sahneler gördüm
    “Genelde” ifadesini bir şekilde teknik olarak doğru olacak biçimde tanımlamak mümkün olabilir, ama o zaman pek bir anlamı kalmaz

• Kod inceleme hakkında geniş iddialarda bulunacaksanız, hangi tür kod incelemeden söz ettiğinizi tanımlamak önemlidir
  Bugün GitHub tarzı asenkron PR incelemesi ve inline yorumlar standart, ama inceleme bundan ibaret değil. Eskiden yüz yüze incelemenin tez savunmasına ya da konferans sunumuna yakın olduğu süreçler de vardı
  Kod incelemenin faydalı bir kalite pratiği olduğuna, hatta gerçekten faydalı az sayıdaki kalite pratiğinden biri olduğuna dair literatür, çoğunlukla bugünkünden çok daha yapılandırılmış inceleme süreçlerinden çıkmıştır
  Kişisel olarak LLM öncesi GitHub tarzı PR incelemeleri, sürecin iyi hissettirmesi ya da governance kutucuğunu işaretlemek içindi; LLM çağında ise maliyet-etkinliği çok daha kötüleşip yok olacak gibi geliyor

  • Senkron inceleme bugün de mümkün. İlk yöneticilerimden biri, “standart” kod inceleme birden fazla tur gidip geliyorsa, neredeyse her zaman yüz yüze görüşmenin ya da taraflardan biri bile uzaktaysa Zoom’da konuyu toparladıktan sonra varılan mutabakatı yorum olarak bırakmanın daha iyi olduğunu öğretmişti
    Zoraki bir teknik benzetme yaparsak, asenkron metin iletişimi başarıyla kodlanabilen bilgi açısından konuşmaya göre daha kayıplı ve throughput’u daha düşüktür. Bu yüzden çok bilgi alışverişi yapmak gerektiğinde senkronizasyon maliyetini ödemeye değer
  • İlk işlerimden birinde değişiklik paketini yazdırıp incelememiz ve imzalamamız gerekiyordu. Son sürümü dosya dolabında saklamaktan sorumlu biri de vardı
    Geleneksel mühendisliğe daha yakındı ve herkesin yazılımı daha kalıcı bir şey olarak düşünmesi gerekiyordu

• Bakımı yapılabilirliği güvence altına almak kod incelemenin faydalarından biri, evet; ama bunun tek amaç olduğunu söylemek oldukça iddialı
  Kod inceleme, ekibin kod değişikliklerini kavramasını ve tüm codebase üzerinde ortak sorumluluğu paylaşmasını sağlayan bir araçtır da

• Kod inceleme tek bir şey değildir. Bilgi paylaşımı, sorumluluk aklama, kod kalitesi, regülasyon uyumu gibi birçok nedeni vardır ve her zaman olduğu gibi hangi amaca sahip olduğu kullanım bağlamına göre değişir

  • Peer review’un nedenini çoğu insanın anlamadığını söylemek epey cahilce görünüyor. Yalnızca tek bir amaç olduğuna inanmak, başka ekiplerle ya da insanlarla çalışma deneyiminin eksik olduğunun işareti gibi görünüyor

• Yazar bir matematikçiyse, “koda bakarak genelde hata bulunamaz” sözü hata bulmanın tamamen imkânsız olduğu anlamına gelmiyor olmalı
  Daha çok tüm hataları bulmanın ya da belirli bir hatayı bulmanın mümkün olmadığı anlamına gelir

  • Üniversitedeki matematik dersi deneyimime göre, matematikçiler bazen diğer insanlarla iletişim kurmakta gerçekten çok kötü olabiliyor. Bu da kendi söylediği şeyle neredeyse herkesin okuma biçiminin neden farklı olduğunu düşündüğünü açıklıyor
  • Anlamı buysa doğru
    Bakımı yapılabilirlik argümanına bağlayarak eklersem, kodu mümkün olduğunca basit hale getirip incelemeyle debug edilebilir olma olasılığını artırmak da incelemenin hedeflerinden biridir. Mutlak anlamda hataları engelleyemez, ama olasılığı artırır
  • Matematikçi olan yazar, kendi doğal dil niceleyicisinin anlamını anlamamış gibi. “Koda bakarak genelde hata bulunamaz”, “koda bakarak genelde herhangi bir hata bulunamaz” anlamına gelir; “tüm hatalar bulunamaz” anlamına değil
    İlk yorum konuyla ilgilidir ama yanlıştır; ikinci yorum doğrudur ama konuyla ilgili değildir
    Muhtemelen yazar “verilen bir hata koda bakarak genelde bulunamaz”, yani “her hata B için B’yi bulmak mümkün değildir” demek istemiştir; ama bu da doğru olmakla birlikte asıl noktadan uzaktır

• PR önerilerini sık sık reddeden kişilerle de, önerileri kabul eden kişilerle de birlikte çalıştım
  Önerileri kabul eden kişinin bir ölçüde sahipliği benimle paylaşmaya istekli olup olmadığını düşünmeye başlıyorum. İkimizin de kodu bakımını üstlendiği, anladığı ve aynı yöne baktığı hissi oluşuyor
  Buna karşılık PR önerilerini reddeden kişinin PR’larına katılma isteğim azalıyor. Nasıl olsa reddedilecekse, neden zaman ayırıp titizlikle inceleyeyim ki

  • Bizim ekipte yorumların başına genelde thought, change, nit, fix, chat gibi önekler koyarız
    Örneğin thought: “ileride foo daha yaygın hale gelebilir, o zaman refactor edelim”; change: “bu sızdıran bir soyutlama, bar gibi modellenmesini isterim”; nit: “isim biraz sezgisel değil, Baz veya Boo’yu düşünelim”; fix: “bu birim testi yanlış alanı doğruluyor”; chat: “bu kategori çözümün şeklini ileride belirleyecek büyük bir karar, önce ekiple konuşalım” gibi
    Bazı önekler, düzeltilene kadar PR’ı durduran şeylerdir; bazıları ise kabul edilse de edilmese de olur türünden yorumlar anlamına gelir. Yazara neyin “ortak bir anlayışa varılması gereken konu”, neyin “tercih ifadesi” ya da “gözlem” olduğu netleşir
    Ancak nit bıraktığınızda karşı taraf katılmayıp görmezden gelse bile gücenmemek gerekir. Eğer güçlü hissediyorsanız, o zaten nit olmamalıydı
  • Önemli olduğunu düşünüyorsanız engelleyici bir öneri bırakıp konuşmayı zorunlu kılmalısınız.