Code Review
Bir yazılım geliştirici bir sorun üzerinde çalışmayı bitirdiğinde, başka bir geliştirici onun kodunu inceler ve aşağıdaki gibi soruların cevaplarını arar:
- Kodda bariz mantık hataları var mı?
- Gereksinimlere bakıldığında, tüm vakalar tam olarak uygulanıyor mu
- Yeni eklenen unit testler, yeni kod için yeterli mi? Koddaki değişiklikleri hesaba katmak için mevcut unit testlerin yeniden yazılması gerekiyor mu?
- Kod, şirketin stil yönergelerine ve standartlarına uygun mu?
Code review ya da kod gözden geçirme, yukarıda bahsi geçen soruların en doğru zaman ve şekilde sorulması ve en nihayetinde de kaliteli bir işin canlı ortama (production) gitmesini sağlamaya çalışır.
Code Review ne zaman yapılır?
Bu amaca yönelik farklı yaklaşımlar vardır. Bu yaklaşımları 3 başlık altında ele alabiliriz. Lemi Orhan Ergin’in aşağıdaki görseline ve tanımlamalarına hızlıca göz atalım.
Collaborative Teams (Paylaşımcı takımlar): Bu takımlarda kurumsal know-how tüm ekipte paylaşılmıştır. Ekip oryante olmuş ve bütüne hakimdir. Eğer mob programming veya pair programming yöntemlerinden bir tanesini uyguluyorsanız, code review genellikle kod yazılırken driver ve navigator’lar arasında olur ve biter. Yine de arada sırada, tüm squad bir araya gelerek, geriye dönük kod gözden geçirme oturumları yapmak, tüm ekip olarak aynı sayfada olmak açısından faydalıdır.
Gatekeepers’ Teams (Bekçinin takımları): Kurumsal hafıza ve koda hakimiyet tek bir ya da bir kaç kişideyse, bu durumlarda code review’ı yapmak da bu kişilerin vazifesine dönüşür. Pull request/merge request mantığını uygulamak gerekir. Birazdan detaylarını aktarmaya çalışacağım.
Group of Silos (Silolaşmış gruplar): Bir ekipte/kurumda herkes yalnızca kendi koduyla uğraşıyorsa, koduna hakim ve ama başkasının kodunu tanımıyorsa bu tür ekiplerde code review yapmak mümkün değildir. Arada sırada bazı şirketlerde durum böyleyken code review yapıyormuş gibi yapıldığına tanık oluyorum ama maalesef pratikte bu uygulama aşağıdaki karikatürdeki durumdan çok da ileriye gidemez.
Kod gözden geçirme, ekibin mevcut iş akışına uygun bir yerde konumlanmalıdır.
Örneğin ekip task branching akışları (her bir task için ayrı branch yaratıp, iş bitince onu uygun bir diğer branch ile merge etmek) uyguluyorsa kod gözden geçirme için uygun zaman kodun merge edildiği an olmalıdır. (Gatekeepers’ teams). Bu yöntemde merge request/pull request açıklaması içerisinde kodu gözden geçirecek kişinin hayatını kolaylaştıracak bilgilerin yer alması çok ama çok önemlidir. Hiç bir ek bilgi vermeden sadece koda bakarak sağlıklı bir kod gözden geçirme yapılması biraz iyimserdir.
Şimdi ideal bir merge request/pull request dokümanı neye benzer biraz ondan bahsedelim.
İdeal bir Merge Request/Pull Request Şablonu
İdeal bir MR/PR içerisinde en azından aşağıdaki ana başlıkların olmasını öneriyorum.
1. Ne?
Bu başlık altında yapılan değişikliğin kısaca ne olduğu anlatılır. Aşağıdaki gibi bir örnek işinizi gayet güzel görecektir.
Kullanıcı girişinde 2FA (two factor authentication)'ı devreye soktum. Tüm veritabanı kontrolleri, rate limiting ve timeout gereksinimleri de JIRA-1345'de tanımlandığı gibi kodlandı.2. Neden?
Burada yazılımcının yaptığı işi neden yaptığını ve hatta eğer mümkünse bu işin ticari gereksinim tarafına da referans vererek anlattığı yerdir.
Bu değişikler ile login mekanizmasını bitirmiş ve hesap yaratma deneyimini tamamlamış oluyoruz. 2FA ile SMS doğrulama ile daha güvenli bir oturum şansı tanıyoruz. Daha fazla detay için #JIRA-1345'e bakılabilir.3. Nasıl?
Burada ihtiyacın kodda nasıl karşılandığı anlatılır. Koda bakarak bariz anlaşılacak kısımları burada not etmeyin. Örneğin recursive bir fonksiyon çağrımı söz konusuysa nedenini, özel bir tasarım şablonu kullanıldıysa neden onun seçildiği gibi, -varsa- faydalı detayları anlatın. Örnek:
Kullanıcı doğrulama için veritabanında yeni bir tabloya ihtiyacım vardı. Bu nedenle two_factor_auths adında yeni bir tablo yaratıldı. Veritabanı migration, model ve controller kodlandı. Şirket içi ORM standartlarımıza bağlı kalındı.4. Testler Hakkında
Burada yapılan değişikliğin nasıl test edilebileceği ile ilgili bilgi verilir. Kodun unit testlerinde neyi nasıl test ettiğinizden bahsedebilir, mocklama yaptıysanız neyi nasıl mockladığınızdan bahsedebilirsiniz. Kimi zaman testleri koşmak zor olabilir. Örneğin Google Cloud PubSub üzerinden iletişim kuran bir kuyruklama uygulamasını koşmak için, önce Pubsub emülatörünü çalıştırmak ve sonrasında testleri koşmak gerekli olabilir. (Bunu mocklamak da mümkün olsa da).
Unit testler tests/blabla/ altında mevcuttur. AWS SQS üzerinden koşan kuyruk yapısını simüle edebilmek için gerekli detaylar README dosyasında mevcuttur.5. Ekran Görüntüleri (seçime bağlı)
Önyüzde yapılan değişiklikleri bir başkasına aktarmanın en iyi yolu ekran görüntüsü almaktır. Uygulamanın eski hali ve yeni halini MR/PR’ınıza eklerseniz ve aradaki değişimi bir iki cümle ile anlatabilirseniz, hiç bir detayın gözden kaçmadığından kesin emin olabilirsiniz.
6. Diğer Detaylar
Diğer başlıklar altına uygmayan ama varsa anlatmak istediğiniz diğer detayları burada anlatabilirsiniz. Burası serbest stil sohbet eder gibi canınız ne isterse kodu gözden geçiren kişiye aktarabileceğiniz detayları içerir. Örneğin:
Burada belki ileride 3. parti bir kimlik doğrulama sağlayıcısı kullanmayı da düşünebiliriz. AWS Cognito iyi bir seçenek olabilir ya da belki Firebase. Değişmesi muhtemel olduğu için burayı bir modül olarak kodladım. Amacım ileride kimlik doğrulama sağlayıcıyı değiştirirsek diğer servislerin hiç etkilenmemesi oldu. Bu bakış açısıyla kodu gözden geçirmeni rica ederim.Merge/Pull Request Şablonları Yaratmak
Yukarıda bahsettiğim pull/merge request şablonunu kurum standartı yapmak ve tüm kullanıcıların bu şablona sadık kalmalarını sağlamak çok iyi bir alışkanlık olacaktır. Bu nedenle;
Gitlab için: https://docs.gitlab.com/ee/user/project/description_templates.html
adreslerine göz atmanızı öneririm.
Code Review Checklist
Bazı kurumlarda kod gözden geçirmede “daha bağışlayıcı” kişiler varken, diğer tarafta da “daha kök söktürücü” kişiler olur. Zor kişilere kod göndermek cesaret isterken, herkesin sevgilisi bir takım kişiler, ne versen onaylar şeklinde şöhret yapabilir.
Bu durumun içinden çıkılmaz bir kalite ve kurum kültürü sorunu yaratacağını öngörmek çok da zor değildir sanıyorum.
Kod gözden geçirme disiplinin kişiden kişiye farklılık göstermemesi için herkesin üzerinde mutabık kaldığı bir checklist hazırlanması gerekir.
Örnek bir checklist aşağıdaki gibi olabilir. Bu listeyi şirketinize ve ihtiyaçlarınıza göre değiştirebilir veya geliştirebilirsiniz.
- Code formatting: Şirketin ve genel kabul gören kod standartlarına uygun bir formatlama yapılmış mı?
- Mimari: Değişikliğin kod mimarisi makul ve mantıklı mı, mevcut mimariye uyumlu mu? Tasarım ve kod şablonlarına uyumlu mu?
- Pratiklere uyum: konfigurasyon dosyalarına alınması gereken şeyler hard-coded mı? Modüler yapı doğru mu tasarlanmış, yeterince comment kullanılmış mı? Testler yeterli ve doğru mu? Logic doğru mu? Healthcheck için doğru parametreleri üretiyor mu? Exception handling doğru mu? Loglama yeterli ve doğru mu?
- Fonksiyonel Olmayan Gereksinimler: Okunabilirlik yeterli mi? (Eğer bir kod zor okunuyorsa, muhtemelen o kod kötü yazılmıştır), Kolay test edilebiliyor mu? Debug edilmeye müsait mi? Konfigurasyon yeterli mi? Doğru şekilde modüllere ayrılmış mı? Tekrar kullanılabilir şekilde mi tasarlanmış? Geliştirilmeye açık mı? Güvenlik sorunu var mı? Performans sorunu var mı? Kullanılabilirlik sorunu var mı?
- Object-Oriented Analysis and Design (OOAD) Prensiplerine Uyum: Single Responsibility Principle (Bir kod parçasına birden fazla sorumluluk yüklemeyin), Open Closed Principle (yeni fonksiyonalite eklerken, mevcut kodu değiştirmekten imtina edin), Liskov substitutability principle (child class, parent class’ın fonksiyonalitesini değiştirmemelidir), Interface segregation (gereksiz uzun interface’ler yaratmayın, anlamlı parçalara bölün), Dependency Injection (bağımlılıkları koda gömmeyin, onları inject eden tasarım şablonlarını kullanın)
Pair Programming uygulanan kurumda kod gözden geçirme yapılır mı?
Pair Programming, birisinin driver diğerinin de navigator diye adlandırıldığı iki kişinin aynı anda aynı kod parçası üzerinde çalıştığı bir disiplindir. Daha kod yazılırken ikinci bir göz incelediği için bazen code review gerekli olmayabilir.
Kod kalitesi göreceli bir konudur. Eğer pair’lar hassas bir kısım üzerinde çalışıyorsa üçüncü ve hatta dördüncü bir gözün yardımına ihtiyaç duyabilir.
Nadiren olsa da, tüm ekibin bir araya gelerek geriye dönük bir kod bloğu üzerinde kod gözden geçirme oturumları yaptıklarına da rastlanır. Şimdi biraz bundan bahsedelim.
Kod Gözden Geçirme Oturumları
Uzun zaman önce, tüm ekiple birlikte yaklaşık ayda bir kez kod inceleme oturumları yapardık. Tüm ekip bir toplantı odasında oturuyorduk ve bir geliştirici son zamanlarda yazdığı zor bir kod parçasını gösteriyor ve neyi neden ve nasıl yaptığını herkese açıklıyordu.
Diğer geliştiriciler, olası sorunları tespit etmeye çalışıyor, kodun nasıl geliştirileceğine dair yorum yapıyor ve önerilerde bulunuyordu. Doğal olarak yazılımcı egoları havada uçuşuyor, senior yazılımcılar juniorları aşağılıyor, herkes birbirine ne kadar zeki ve yetenekli olduğunu ispatlamak için fırsat kolluyordu. Nihayetinde tüm ekip birbirimizden nefret eder şekilde odadan çıktığımız oturumların sayısı hiç de az değildi.
Bugün artık herhangi bir takımın bu yöntemi kalıcı olarak kullandığını düşünmüyorum. Ancak;
- bir kodun ekipçe devir alınması,
- az kişide birikmiş kurumsal hafızanın tüm ekibe dağıtılması,
- uzun zamandır ortada kalmış bir teknik borcun giderilmesi için aksiyon planı hazırlanması,
- içinden çıkılamamış bir durumun tüm ekipçe ortak zeka ile çözülmesi
gibi istinai durumlarda anlamlı olabilir. Ki zaten tüm takımın bir odada toplanarak onların değerli zamanlarını çalmanın çok da efektif bir yöntem olduğunu düşünmüyorum.
Otomatik Kod Gözden Geçirme Araçları
CI/CD akışı içerisinde otomatik kod gözden geçirme araçları kullanımı bu konuyu ciddiye alan şirketlerde sıklıkla uygulanan bir yöntemdir. Bu yöntem kesinlikle, buraya kadar anlattığım kod gözden geçirme disiplinlerinin bir alternatifi değil, tamamlayıcısıdır. Bugün piyasadaki hiçbir kod analiz aracı bir insanın gözünden bakıldığı kadar detaylı inceleme yapamaz. Belki ileride mümkün olabilir ama bugün değil.
Bu amaçla CodeBeat, DeepSource, Code Climate, Codacy, VeraCode göz atmak isteyebileceğiniz ürünlerden sadece bir kaçı. Unutmayın ki bu alanda sürekli yeni ürünler çıkıyor veya mevcut ürünler hızlı bir şekilde rüzgarı kaçırabiliyorlar. O nedenle bir üründe karar kılmadan önce iyice araştırıp, kendi muhakemenizi kullanmanızı öneririm.
Kod Gözden Geçirmeyi Şirketin Akışına Sokmak
Kod gözden geçirmeyi bir kurumunun akışının zorunlu bir parçası yapmanın en temel yolu, görev yönetim aracında hangi ürün kullanılıyorsa, o ürün akış workflow’unda “Code Review” diye bir aşama (isim farklı olabilir) koymaktır.
Örnek bir Jira akışı aşağıdaki gibidir.
Bu tür bir akış tasarlandığında, Jira’da aşağıdaki gibi bir ekranda Code Review status’u görünecek ve Code Review aşaması geçilmeden görev Tamamlandı (Done) statüsüne geçemeyecektir.
Code review ile ilgili yazılacak o kadar çok şey var ki, burada en önemlilerine yer vermeye çalıştım. Eğer konuyla ilgili daha fazla şey okumak isterseniz sırada Lemi Orhan Ergin’in “Code Review esnasında yapılmaması gereken 10 şey” başlıklı sunumunu önerebilirim.
