Unified Diff ve Split Diff Karşılaştırması
(matklad.github.io)- Küçük değişiklikler hem unified diff hem de split diff ile yeterince incelenebilir; ancak büyük ve karmaşık değişikliklerde yalnızca diff okuyarak yapılan inceleme sınıra dayanır
- Büyük değişiklikler, değişiklik geçmişinden ziyade belirli bir andaki kod tabanının genel durumu temel alınarak, gerçek kodu düzenler gibi gezilip doğrulanmalıdır
- İdeal inceleme ekranı, diskteki mevcut kod ile ilgili alanın unified diff görünümünü yan yana göstererek kod bağlamını ve değişiklikleri aynı anda takip etmeyi sağlamalıdır
- Mevcut araç desteği yetersiz olduğundan, PR'ı lokale checkout edip commit'leri silerek yalnızca değişiklikleri bırakan düşük teknolojili bir iş akışı kullanılıyor
- Bu yöntemle değişen dosyalar arasında gezinmek ve reviewed hunk işaretlemek mümkün olsa da, editördeki mevcut konum ile diff otomatik olarak eşitlenmediği için hâlâ manuel işlem gerekiyor
Diff'ten ziyade mevcut kod durumunu incelemek
- split diff ve unified diff, küçük ve basit değişikliklerde ikisi de iyi çalışır
- Değişiklik büyüyüp karmaşıklaştığında yalnızca diff okumak yeterli değildir
- Büyük değişikliklerde, kod tabanının belirli bir andaki durumuna düzgünce bakmak gerekir
- Yakın zamanda değişen alanlara dikkat etmekle birlikte, temelde normal kod incelemesi gibi bakılmalıdır
- Testler çalıştırılmalıdır
- goto definition gibi editör gezinti özellikleri kullanılmalıdır
- Yerel değişiklikler uygulanarak başka bir yazım biçiminin mümkün olup olmadığı kontrol edilmelidir
- Daha geniş bağlamda değişmiş olması gereken yerler aranmalıdır
- Değişiklik geçmişinden bağımsız olarak mevcut kod tabanında tuhaf görünen noktalar fark edilebilmelidir
İstenen inceleme ekranı ve yerel geçici çözüm
- İdeal diff görünümü, solda mevcut kod durumunu, sağda ise şu anda görünen kod alanının unified diff'ini birlikte gösteren bir yapıdadır
- Soldaki kod, diskteki mevcut durumla aynı olmalıdır
- Değişiklikler kenar boşluğunda hafifçe gösterilir
- Sağdaki diff, solda o anda görünen kod alanıyla eşleşmelidir
- Bu tür bir inceleme biçimi için mevcut araç desteği yetersizdir
- Gerçek iş akışı, gpr script ile PR'ı lokale checkout etme şeklindedir
- Son aşamada PR'ın commit'leri silinir ve yalnızca değişiklikler bırakılır
- Bu durumda staging ve commit iş akışı kod incelemesi için kullanılır
- edamagit ile değişen dosyaların listesi görülebilir ve editörde sonraki/önceki değişikliğe gidilebilir
- staging area kullanılarak daha önce incelenmiş hunk'lar da işaretlenebilir
- Geriye kalan rahatsızlık, magit status buffer ile editörde o anda açık olan dosya arasında otomatik eşitleme olmamasıdır
- Mevcut dosyayı ve diff'i yan yana görmek için diff'i manuel olarak açmak ve o anda bakılan konuma kadar elle kaydırmak gerekir
- Koda bu kadar yakın duran bir inceleme yapmak için şu anda geçici araçları bizzat oluşturmak gerekiyor
- Kod incelemesinin temel amacı her zaman yalnızca kod denetimi olmak zorunda değildir; ilgili yazı olarak Two Kinds of Code Review da okunabilir
1 yorum
Hacker News yorumları
Büyük değişikliklerde diff incelemesi değil de, belirli bir andaki kod tabanını düzgünce inceleyip son değişen alanlara özellikle dikkat etmek istemek bana şahsen genel kod inceleme anlayışıyla çok uyumlu gelmiyor
Takıma ve ticket'a göre değişir ama, inceleyen kişi tüm koddan esasen sorumlu olan principal engineer benzeri bir rolde değilse, kod incelemesi genelde ikinci bir göz olarak yapılan basit bir tutarlılık kontrolüne daha yakındır diye düşünüyorum
PR içindeki commit'ler iyi düzenlenmiş ya da squash edilmişse artımlı değişiklikleri ayrıca incelemek de nispeten kolay olur
Sonuçta şikayet daha çok inceleme türleri arasındaki farka benziyor. Yaygın olan tutarlılık kontrolü incelemesi ile derin mimari/işlevsel pre-merge incelemesi var; web araçlarındaki kod incelemesi çoğunlukla ilkine daha yakın görünüyor
İyi bir kod incelemesi iyi bir PR ile başlar. PR, amacını ve bu amaca nasıl ulaştığını açıklamalıdır
İyi bir incelemenin ilk maddesi kodun açıklanan hedefe ulaşıp ulaşmadığını doğrulamak, ikincisi iddia edilen işlevi sınayan testlerin olup olmadığını görmek, üçüncüsü mevcut sistemin mimari teamüllerine uyup uymadığını kontrol etmek, dördüncüsü stilin beklentilere uygun olup olmadığına bakmak ve son olarak API kullanımını daha iyi hale getirecek öneriler olup olmadığını değerlendirmektir
Sadece tutarlılık kontrolü yapan kod incelemesi, CI'ın da yapabileceği bir iş olduğu için pek faydalı değildir. Kod incelemesi insani bir süreçtir; bu yüzden insanların yapabileceği şeylerden mümkün olduğunca yararlanmalı, geri kalanını makinelere bırakmalıyız
Bu maddelerin içinde örtük olarak şu soru da vardır: “Bu değişiklik gelecekteki katkılar için iyi bir örnek olur mu?”
Kod incelemesini hep tester bakış açısından yaptım. Kodu derleyip çalıştırarak amaçlandığı gibi davranıp davranmadığına bakıyor, zayıf noktaları arıyor ve bozmaya çalışıyorum
Diff son değil, başlangıçtır. Sadece kodun neresini kurcalamak gerektiğini gösteren bir ipucudur; asıl kanıt gerçek davranıştan gelir
“Bu tester'ın işidir, mühendisin değil” denebilir ama, kodu anlayan ve nereye bakması gerektiğini bilen birinden daha iyi kim test edebilir ki?
Bu yaklaşımın azınlıkta olduğunun farkındayım. Birçok mühendis, kodu bizzat çalıştırmaya, hatta debugger kullanmaya bile garip biçimde direnç gösteriyor gibi görünüyor. Her şeyi otomatikleştirmek için devasa altyapılar kuruyorlar ama otomasyon da sonuçta hata içerebilen bir koddur. Peki testleri kim test ediyor, Quis custodiet ipsos custodes?
Kod incelemesi, CI hariç son savunma hattıdır ama çalıştığım ekiplerde PR'ın getirdiği büyük yönelim o noktadan önce zaten birkaç kişiyle tartışılmış olurdu. Eşli çalışma, derin açıklamalar, kahve sohbetleri; karmaşıklığa uygun hangi yöntemse o yeterli
Böyle olunca PR, yeni modül ayırma, büyük bir bağımlılık ekleme ya da API yüzeyini yeniden işleme gibi stratejinin kendisini değil, o stratejiyi hayata geçiren taktikleri inceleme yeri olur
Önceden iletişim olmadan kodun sadece bir kısmında tutarlılık kontrolü yapmak, kod sahipliğinin parçalanması riskini doğurabilir. “Şu modül ne yapıyor?” “Bilmiyorum, Bob'a sor” gibi bir duruma dönüşür
Bundan bağımsız olarak, inceleme modu ne olursa olsun yazarın önerdiği diff görünümü fikri hoşuma gitti
Kırmızı ve yeşil satırların iç içe geçtiği bloklarla herhangi bir inceleme yapmak zor. Bu, kodu görmenin faydalı bir yolu değil; en azından varsayılan görünüm olarak uygun değil
Yine de iş olduğu için bir şekilde yapıyoruz ama yazarın söylediği gibi bunun pek kullanışlı bir görünüm olmadığına %100 katılıyorum
Hata, özellik vb. konuları ele alan temel bir süreç olmayınca, her mühendis bir özelliğin işbirliği içinde nasıl yayınlanacağını her seferinde yeniden icat etmek zorunda kalıyor
Çoğu kişi neredeyse hiçbir şey yapmıyor ve kod incelemesi, bu konuda anlamlı ayrıntı düzeyinde yürütülen tek işbirliği haline geliyor. Böyle olunca da mümkün olan her müdahale tek bir ana sıkıştırılmaya çalışılıyor
Üçüncü ya da dördüncü bir seçenek olarak difftastic de anılabilir. Satır bazlı diff yerine “yapısal” diff kullanarak değişiklikleri daha ayrıntılı vurguluyor
https://github.com/Wilfred/difftastic
Hatırladığım kadarıyla bazen diff'i yaklaşık %35 daha okunur hale getiriyordu, genelde %60 oranında fark yaratmıyordu ve nadiren %5 oranında daha zor okunur hale getiriyordu
Bunu birkaç yıl önce kullandım, bu yüzden somut sorunları pek hatırlamıyorum. Kullanmayı bırakmamın tek nedeni Git diff için magit kullanmaya başlamış olmamdı
git diff --word-diffiçinde de buna benzer bir özellik var ve oldukça iyiVim ile incelerken de benzer hissediyorum
PR açmak için ufak bir betik var; değişen dosyalar için kabaca
vimdiff <(git show baseref:file) filegibi çalışıyor. Vim sekmeleri aslında birer görünüm olduğu için, tek tek buffer'ları farklı durumlarda aynı anda açık tutabilmek güzelVarsayılan görünümde kaydırma kilidi beklendiği gibi çalışıyor, gerekirse ayrı sekmelerde bundan kaçınılabiliyor
[cve]c, çalışma dizinine göre değil PR'daki değişiklik kümesine göre hunk'lar arasında geçiş yapıyordpvedgile hunk'ları salt okunur diff buffer'ına push/pull edip “tamamlandı” diye işaretleyebiliyorsunuz; sonra da canlı buffer'daki vurgulanmış değişikliklerden gizleniyorCanlı buffer'da bir değişiklik doğrudan commit edilebiliyor ya da yorum olarak push edilebiliyor
Go to definition, build entegrasyonu, açılır belge pencereleri gibi normal editör özellikleri de mevcut ağaç durumunda aynen çalışıyor
Bu yaklaşım, PR'ın temel branch'e göre değişikliklerini görürken çalışma dizinini temiz tutuyor. Çalışma dizinini kirletilmiş durumda değişikliklere bakan matklad'ın çözümünden çok daha iyi görünüyor
Çalıştığım ortamda değişiklikler sık sık
--fixupcommit'leri olarak ekleniyor ve araç merge anındagit-interpret-trailersçağrısıyla fixup commit yazarını asıl commit'e yeniden bağlayacak şekilde bunları tekrar birleştiriyor; bu yüzden özellikle iyi. Bazen PR'daki metin yorumlarını çıkarıp uygunsaReviewed-bytrailer'ı ekliyor, ya da +1 anlamına geliyorsaAcked-bytrailer'ı ekliyorÜzerinde çok sayıda mühendisin çalıştığı çok büyük ve karmaşık kod tabanlarında bu büyük bir sorun
Kod incelemesinin zor olmasının nedeni, diff'in her zaman makul görünmesi, testlerin her zaman geçmesi ve temel şeylerin hep kontrol edilmiş olması
Ama değişiklik makul görünse bile gerçekte yanlış olduğu durumlar sık yaşanıyor
Makul görünen tek bir kötü değişiklik bile tüm mimarinin sürüklenmesine yol açabilir
Her zamanki gibi bu da teknik olarak bir araç sorunu değil; daha çok kültür ve bilgi paylaşımı sorunu. Daha iyi araçlar tek başına bunu çözmez
Kod incelemesinde bizzat bug bulmak nadirdir. Bunun yerine, böyle bug'ları yakalayacak testlerin olup olmadığına bakarım
Bir şeyi kaçırıyor gibiyim. Yazar split diff'in kendisine uymadığını söylüyor ama nedenini açıklamıyor
İdeal diff, yinelenen bağlamı çıkarılmış split diff'e neredeyse benziyor. Bağlamın iki tarafta da tekrar etmesi yerine yalnızca solda bulunuyor
Sağ panelde yinelenen bağlamı kaldırmanın ne fayda sağladığını merak ediyorum
Kullandığım editör/IDE'lerde diff içindeki bu tür destekler genelde çok temel düzeyde oluyor. Go to definition bazen yalnızca aynı dosya içinde çalışıyor, otomatik tamamlama yeni eklenen öğelerde iyi çalışmıyor ve çoğu zaman refactoring özellikleri bulunmuyor
Benim anladığım kadarıyla yazar diff'i tersine çevirmek istiyor. Değişikliğin kendisine bakmak yerine koda bakıp orada ilgili bir değişiklik olup olmadığını görmek istiyor
Yazı ayrıntı açısından zayıf ama görünüşe göre implementasyonlara ya da çağrı noktalarına gidip uygun değişikliklerin yapılıp yapılmadığını kontrol etmek istiyor
Diff vurguları sanılandan daha dikkat dağıtıcı olabiliyor; kapatıldığında okunabilirlik belirgin biçimde artabiliyor. Ben de bazen diff görüntüleyicide bunu kapatıp koda taze gözle bakıyorum, ama birleşik diff'i split pane içinde birlikte gösterme seçeneği yok
GitHub'da
.tuşuna basınca tarayıcı içindeki tam IDE'ye giriliyorİnceleme yaparken çok faydalı oldu. Parçalara bakmak yerine değişiklikleri dosyanın tüm bağlamı içinde görebildiğim için ince tasarım sorunlarını çok daha iyi yakalıyorum
Bu arada bu kısayol GitLab'da da çalışıyor
p4merge'ün bazı özelliklerini özlüyorum. Örneğin split diff'ler arasındaki boşlukta, dosyanın hangi bölümünün başka dosyanın hangi bölümüyle eşleştiğini gösterirken iki tarafta da diff etkisini birlikte gösterebiliyor: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
3-way merge özelliğini de seviyorum: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
Bu özellikler yaygın kullanılan web tabanlı diff araçlarına gelmedi. Zor merge sonuçlarını göstermek için 3-way diff'in iyi bir yöntem olduğunu düşünüyorum
GitHub kullanıyorsanız github.com'daki pull request sayfasında
.tuşuna basabilir ya da alan adınıgithub.devolarak değiştirebilirsinizBöylece tarayıcıda VSCode açılır ve pull request diff görünümü ile gösterilir
Avantajı, dosyanın tamamını görebilmeniz ve diff algoritmasının github.com'dakinden farklı olması nedeniyle karmaşık diff'lerde bazen daha okunaklı olması
PR yorumlarını satır dışı bir liste olarak gösteren side-by-side diff görünümü çok yararlı olabilir ama bunun için tarayıcıda tam bir VSCode örneği açmak zorunda olmak can sıkıcı
Aslında o kadar yararlı ki, GitHub'da eskiden buna benzer bir split view vardı; insanları AI/ML/crypto/blockchain/cloud-enabled geliştirme ortamları gibi moda sözcük yığınlarına itmeye gerek yoktu
Fikir iyi ama uygulama berbat. Hatta bugünlerde GitHub'ın genelini özetleyen bir ifade gibi geliyor
Meld bu tür kullanım için oldukça uygun
https://meldmerge.org/
En sevdiğim araç diff2html-cli: https://diff2html.xyz/
https://www.npmjs.com/package/diff2html-cli
diff'i HTML olarak görüntüleyebiliyor ve yan yana ya da unified biçimini destekliyor