- GitHub tarzı PR incelemesi, özgün değişikliğin üstüne düzeltme commit’lerinin kolayca yığılmasına yol açar; bu da inceleme süreci ile nihai kod geçmişinin birbirine karışması sorununu doğurur
- İyi bir yama serisi; refactoring, yeni API ekleme ve mevcut kullanıcıları taşıma gibi mantıksal olarak ayrılmış commit’lerin sırayla okunabilmesini sağlamalıdır
"fix review" ya da "minor" gibi commit’ler biriktikçe değişikliğin amacı bulanıklaşır ve git blame ile git bisect kullanarak sorunun kök nedenini bulmanın maliyeti artar- interdiff incelemesi, yama serisinin yeni bir sürümünü yayımlar ve commit bazındaki değişiklik farklarını
git range-diff main..v1 main..v2 ile karşılaştırır
- İnceleyen kişi 500 satırın tamamını yeniden okumak yerine yalnızca 50 satırlık artımlı farkı kontrol edebilir; yazar da
"address review" gibi gürültü commit’leri olmadan nihai commit yapısını koruyabilir
Kod inceleme araçları ve başlangıç noktası
- Gerrit Code Review, Git deposuyla birlikte çalışan bir açık kaynak kod inceleme aracıdır; yamaların gönderilmesini, başkalarının kodu incelemesini ve yorum bırakmasını destekler
- Kod inceleme yöntemleri Gerrit, GitHub ve Phabricator’la sınırlı değildir; hata takip sistemine
.patch dosyası yüklemekten git send-email kullanmaya, bir meslektaşla uzun tartışmalar sonunda implementasyonu yönlendirmeye kadar çeşitlidir
- Go programlama dili Gerrit kullanır, KDE ve LLVM Phabricator kullandı; Jujutsu ise bir dönem Gerrit kullanımını değerlendirdi
- Linux çekirdeği e-posta tabanlı iş akışını kendine özgü bir şekilde kullanır ve birçok proje, kullanımı kolay ve başlangıç maliyeti düşük olduğu için GitHub’ı seçer
- Asıl sorun, araçların aynı inceleme modelini sunmaması ve GitHub’ın varsayılan deneyiminin her zaman yeterli olmamasıdır
İyi bir yama serisinin koşulları
- İdeal bir yama serisi, tek bir büyük değişikliği mantıksal adımlara bölerek sunar
- Önce tesadüfen fark edilen bir kod temizliği yapılır
- Sonra yeni bir API eklenir
- Son olarak mevcut API kullanıcıları yeni API’ye taşınır
- Her yamanın bağımsız bir gerekçesi olmalı ve sırayla uygulandığında kodun adım adım nasıl evrildiği görülebilmelidir
- Bu yapı yalnızca yazar ve inceleyen için değil, daha sonra
blame ya da bisect ile değişikliğin nedenini araştıracak bakımcılar için de avantaj sağlar
- Gerçek yama boyutu duruma göre değişir
- Refactoring olmayabilir
- 100 satır olabilir
- Temel bir API değişikliği gibi tüm çağrı noktalarının düzeltilmesi gerekiyorsa 500 satır da olabilir
GitHub tarzı “diff soup”un oluşturduğu karışık geçmiş
- GitHub, inceleme yorumlarını yansıtırken özgün commit’lerin üstüne yeni commit’ler ekleme akışını açık ya da örtük biçimde teşvik eder
- Örnekte, özgün 3 commit’in ardından sırasıyla Alice ve Bob’un inceleme geri bildirimleri, test ekleme ve
"minor" implementasyon değişikliği commit’leri birikir
- Commit grafiğinde yalnızca ebeveyn-çocuk ilişkileri kaldığı için, inceleme düzeltme commit’lerinin özgün commit’lerdeki hangi sorunu düzelttiği görünmez
- Tek bir
"fix review" commit’i birden fazla özgün commit’e aynı anda dokunursa kavramsal model daha da karmaşıklaşır
- Bir düzeltme commit’inin tam olarak tek bir geri bildirimi yansıtması sağlanabilir ama bu kez fixup commit sayısı artar
- Büyük değişiklik paketiyle inceleme düzeltmelerinin aynı kapta karıştığı bu durum diff soup olarak adlandırılır
git blame ve git bisect sinyalinin bulanıklaşması
git blame satır düzeyinde çalıştığı için, inceleme yansıtma commit’i mevcut satırın küçük bir bölümünü bile değiştirirse satırın kaynağı "fix alice review" gibi bir commit olarak görünebilir- Gerçek değişiklik nedenini bulmak için özgün commit ile inceleme düzeltme commit’lerini yeniden takip etmeyi gerektiren bir commit arkeolojisi gerekir
git bisectte de "minor" commit’inin gerçekten kök neden olup olmadığı ya da hatanın yeni API ilk eklendiği anda var olup sonradan mı görünür hâle geldiği belirsizleşebilir- Yeni API büyük bir performans artışı sağlıyorsa geri almak istenmeyebilir ya da mümkün olmayabilir
- Regresyon haftalar sonra ortaya çıkarsa, P1 toplantısı gibi acil bir durumda yalnızca
"minor" commit mesajına bakıp ek araştırma yapmanın maliyeti doğar
- İncelemenin yalnızca iki tur sürdüğü örnekte bile geçmiş karmaşıklaşır; açık kaynak projelerde olduğu gibi çok sayıda inceleme turu olduğunda sorun daha da büyür
GitHub UX’in bu akışı güçlendirme biçimi
- GitHub’da artımlı inceleme yapmak için üstüne yeni commit ekleme yaklaşımı fiilen en kolay yoldur
- İnceleyen için yeni API’nin 500 satırını baştan okumaktansa
+/- 10 satırlık bir "fix alice review" yamasına bakmak daha az zahmetlidir
- PR’nin diff sekmesi varsayılan olarak tüm commit’leri tek bir diff içinde birleştirerek gösterir
- Bu,
git diff master..foo-branch görüntülemeye benzer
- İnsanların değişiklik yazma ve okuma biçimiyle çok iyi örtüşmez
- GitHub UX, adlandırılmış branch modeline güçlü biçimde uyumludur; bu da branch dışındaki birimler arasındaki farkları görmeyi zorlaştırır
- Force push sonrasında PR’deki
Force Push satırındaki Compare düğmesiyle diff görülebilir
- Ancak bu yalnızca önceki branch head’i ile mevcut branch head’i arasındaki tam diffi gösterir
- Daha ayrıntılı karşılaştırma için commit kimliklerini elle bulup URL’ye eklemek gerekir
interdiff incelemesi ve git range-diff
- interdiff incelemesi, özgün 3 commit’in üstüne düzeltme commit’leri eklemek yerine aynı 3 commit’ten oluşan yeni bir sürüm yayımlar
- Örneğin
v1 yama serisine Bob’un yorumlarını yansıtmak gerekiyorsa, birinci ve üçüncü commit düzenlenerek v2 serisi oluşturulur
- Sonrasında Alice’in yorumları, test ekleme ve
"minor" değişikliği de aynı şekilde yansıtılsa bile nihai sonuç yine 3 commit olarak kalır
- Eğer
v1deki A, B, C commit’leri v2deki X, Y, Z commit’lerine evrildiyse, git range-diff her konumdaki commit’ler arasındaki farkı gösterir
git range-diff \
main..v1 \
main..v2
- Bu komut,
v1 branch’indeki 3 commit ile v2 branch’indeki 3 commit arasındaki eşleştirilmiş diffleri gösterir
- Birinci commit
diff(A, X)
- İkinci commit
diff(B, Y)
- Üçüncü commit
diff(C, Z)
interdiff yaklaşımının pratik etkisi
- GitHub genelde
diff(main, C) ya da diff(main, Z) gibi tüm branch diff’ini gösterirken, interdiff commit sürümleri arasındaki farkı gösterir
- İnceleyen kişi daha önce gördüğü 500 satırlık API değişikliğini yeniden okumak yerine yalnızca değişen 50 satırı kontrol edebilir
- Yazarın geçmişte
"address review" gibi 30 gürültü commit’i biriktirmesi gerekmez
git blame, satırları daha az gürültüyle özgün değişiklik commit’lerine bağlama olasılığını artırırgit bisect, regresyonu yeni API commit’i gibi daha anlamlı birime işaret etme olasılığını artırır- Sonuç olarak temel tanılama araçlarının sinyal-gürültü oranı iyileşir
Birleştirme stratejisi ve rebase hakkında notlar
- Yama birleştirme stratejisine ilişkin açıklama TODO olarak bırakılmıştır
- Yaklaşım, başkalarının commit’lerini bunun üstüne kurmasının beklendiği herkese açık bir branch söz konusu değilse
git rebase kullanımının sorun olmadığı yönündedir
- interdiff inceleme sistemi, daha küçük ve daha hızlı birleştirilebilen yamaları teşvik eder
- 5 commit’in tamamı hazır olana kadar beklemek gerekmez
- İlk 3’ü uygunsa ve son 2’si daha fazla çalışma gerektiriyorsa yalnızca bu 3’ü birleştirmek mümkündür
- Amaç, insanların uzun ömürlü branch’ler yerine main branch’i temel alarak çalışmasını sağlamak ve birden çok uzak branch’in birbirine dolanarak birleştirilmesi durumunu azaltmaktır
- Linux çekirdeği gibi bazı projelerde herkese açık branch’ler başka depolarla birleştirilir ya da başka branch’lerin üstünde açıkça yayımlanır
- Linux geliştiricileri yama serilerini oluşturup cilalarken
git rebase kullanabilir
- Ancak herkese açık branch oluşturulduktan sonra bu branch bir başkasının ağacına birleştirilebileceği için force push yapılmaz; bunun yerine yeni commit’ler içeren yeni bir branch oluşturulur
1 yorum
Hacker News yorumları
GitHub’da genel olarak buna benzer bir akış kullanıyorum; dezavantajı bana daha fazla iş çıkarması ve işbirlikçiler için sezgisel olmaması
Yine de gözden geçiren kişinin yalnızca kendi geri bildirimlerinin yansıtıldığı farkı görebilmesi,
git blamevegit bisecti bozmaması gibi avantajlar olduğu gibi duruyorİnceleme geri bildirimini işlerken
git commit --fixupile commit atıyor, push ettikten sonra inceleme yorumuna yanıt olarak fixup commit hash’ini bırakıyorumPR onaylanıp merge’den hemen önceki aşamaya gelince
git rebase --interactive origin/main --autosquashçalıştırarak fixup commit’leri asıl commit’lerle birleştiriyor, son olarakgit push --force-with-leasesonrası merge ediyorumİnceleme bitmeden force push yaparken dikkatli olmak gerekiyor. Bunu yaparsanız reviewer son incelemesinden sonra eklenenleri göremez hale gelir
Terminal otomatik tamamlamasına epey bağımlıyım;
git rekadar yazınca bile uzun komuta ulaşabiliyorum, ama biraz hantal. Bu akışı destekleyen ve teşvik eden bir araç olsa iyi olurdu. Yine de GitHub’a bağlıysanız bu kadarı fena değilAncak pratikte daha çok Git’in ileri seviye özelliklerini bilinçli olarak öğrenen, disiplinli mühendislik ekiplerinde kullanılıyor
~/.gitconfigiçine aşağıdaki ayarı koymak yeterli[rebase] autosquash = trueBöylece sonrasında yalnızca
git rebase -i origin/mainile fixup/squash commit’leri otomatik olarak yeniden sıralanır. Küçük bir ayar ama iş akışını ciddi biçimde iyileştiriyorÜzerinde birden fazla branch bulunan commit serilerini rebase ederken
--update-refsgerekmesine benzer bir alan bu. İnsan neden graf ilişkilerini takip etmek, commit’leri elle yerleştirmek ve branch’leri taşımak zorunda kalsın ki? Bilgisayarlar graflarla iyi başa çıkar; işi onlara bırakmak gerekirgit absorbda var ama Sapling uygulaması kadar sağlam değil[1]Asıl sorun etkileşimli rebase kullanılıp kullanılmaması değil; çoğunlukla inceleme aracının kendi kullanıcı deneyimi ve bunun teşvik ettiği döngüsel yapı. Örneğin fixup commit’leri, GitHub’ın referans noktaları arasındaki farkları gösterme sorununu çözmez; referans noktası büyükse incelemeyi tamamen mahvedebilir. Örneğin 10 yeni commit’in üstüne rebase ettiğiniz durum böyle
Git’in kendi UX’inden de şikâyetçiyim ama orijinal yazı daha çok GitHub’a yönelik bir şikâyet gibi
[1] İki temel algoritma arasındaki farkı gösteren örnek bu GitHub issue’sunda var: https://github.com/martinvonz/jj/issues/170
Bu yöntemin ideal olduğuna %100 katılıyorum. GitHub yöntemi gerçekten berbat ve bu kadar çok insanın bunu normal kabul eder hale gelmesi trajik
Eskiden Phabricator ile bunu böyle yapardık; tüm incelemeleri tek seferde güncelleyen komut satırı makrosu sayesinde bir ölçüde elle yapılabiliyordu. Yine de açık bir UI olsa daha iyi olurdu
Evet. Kafamda gerçek kod inceleme tarzı olarak hayal ettiğim şey tam olarak buydu; GitHub’ın yaptığı yöntem değil. Bunun bir adı olduğunu öğrenmek güzel
Ek olarak, inceleme sisteminin hazır yamaları incelemenin “dışına” itebilmesi iyi olurdu. Büyük bir özellik üzerinde çalışırken yapılan küçük bir hata düzeltmesi küçük ve bağımsız bir yama olmalı; reviewer ile hızlıca uzlaşılması muhtemel
O zaman o yamayı tüm seriden çıkarıp main’e cherry-pick etmek ve incelemeyi yeni HEAD üzerine rebase etmek isterim. Ya da güncel yama serisini main üzerine rebase edip uzlaşılan yamayı en başa yeniden sıraladıktan sonra main’i o yamaya fast-forward etmek de mümkün olabilir
Esasen inceleme kapsamını “hâlâ tartışılmakta olan kısım” ile daraltıp, hata düzeltmelerinin hazır olur olmaz merge edilmesini sağlamak anlamına geliyor
Karşı argüman “bunu ayrı bir inceleme/PR yap” olacaktır; ama o zaman patch set A’nın patch set B’ye bağımlı olması, B merge edildikten sonra ise main’e bağımlı hale gelmesi gibi karmaşıklıklar doğuyor
Burada Attention Set gibi akıllı UX fikirleri de rol oynuyor. Temelde “sonraki eylemi yapması gereken kişi kim?” sorusunu gösterir ve sıra tabanlı bir oyun gibi çalışır. İncelemeyi az önce yaptıysanız o patch’in attention set’inde artık siz değil, yazar vardır
Bu yüzden Gerrit UI’da o öğe kuyruğunuzun altlarına iner. Kuyruğun en üstünde attention set’inde sizin bulunduğunuz öğeler vardır. İşler doğal olarak bu şekilde gruplanır
GitHub UX’indeki diğer sinir bozucu küçük sorunların hepsine değinmedim ama pull request listesi bile alternatiflerden kötü. Hangi öğenin hangi durumda olduğunu anlayamazsınız ve sonunda her şeyi okumanız gerekir
En alttaki x ise doğrudan submit edip kalanlarla devam edebilirsiniz
Kod incelemeye yönelik yeni yaklaşımlar görmek her zaman ilginç. GitHub’ın da avantajları var ama mükemmel olmaktan çok uzak
Verilen senaryoda, üç yamayı birbirine bağımlı ayrı pull request’lere bölmeyi düşünebilirsiniz. GitHub bunu varsayılan olarak desteklemiyor ama uygun bir kod inceleme aracı, pull request’leri küçük tutarken bağımlılıkları da yönetebilmelidir. Örneğin yama 3’ün yama 2’ye, yama 2’nin de yama 1’e bağımlı olmasını sağlayabilirsiniz
Aracın sunduğu bağımlılık takibi sayesinde, gerekirse hepsinin birlikte dağıtıma çıkması güvence altına alınabilir
Her yamayı ayrı incelemek geri bildirimi daha net ve yanıtlamayı daha kolay hâle getirir. Pull request içinde commit’leri squash ederek tek tek değişiklikleri doğru yansıtan temiz bir commit geçmişi de oluşturabilirsiniz. Daha da ötesi, uygun bir araç pull request’leri ve incelemeleri yapay zeka ile özetleyerek, elle uğraşmadan doğru commit mesajları yazmayı da kolaylaştırabilir
İyi bir kod inceleme aracı rebase, merge, force push gibi git işlemlerinin etkisi altında kalmamalıdır. Arkada ne kadar karmaşık git işlemi olursa olsun, inceleyen kişi yalnızca son incelemeden bu yana olan değişiklikleri görmelidir. Böylece büyük diff’i yeniden incelemek yerine yeni içeriğe odaklanabilir ve inceleme geçmişi commit geçmişinden ayrılarak temiz kalır
Pull request’leri bu şekilde bölüp karşılıklı bağımlılıkları takip etmenin gereksinimi karşılayıp karşılamayacağını merak ediyorum
Kelimenin tam anlamıyla git’in desteklemek üzere tasarlandığı iş akışı bu ve GitHub pull request’lere sahip olduğundan beri dile getirilen eski eleştirinin özü de bu
Bu fikrin, takvime bakınca 15 yılı aşkın süre boyunca “mailing list eskilerinin” yöntemi diye küçümsendikten sonra neden ancak şimdi moda geliştirici kültürlerine yükseldiğini merak ediyorum
Pek çok yazılım geliştiricinin en çok kullandığı araçlardan birini doğru düzgün öğrenmeyi uzun süre reddettiği için kasıtlı olarak kötü iş akışları seçtiğini düşünüyordum. Bu yüzden şirketi git’e geçirirken böyle araçları ve akışları seçip geliştirdim. Ne de olsa IBM satın aldınız diye kovulmazsınız
Dış katkıda bulunan biri depo fork’u kullanıyorsa, üç PR sırasıyla A, A+B, A+B+C’yi kademeli olarak içerir. Bu durumda son iki PR’da, başka PR’da zaten incelenmiş kodu tekrar diff olarak görmek gerekir ve bu da incelemeyi zorlaştırır
Her commit bağımsız olarak incelenebilir. Commit’ler arası bağımlılık takibi de yapılabilir. Yerleşik yapay zeka olmaması da iyi bir şey. PR’ların aksine commit’ler arasında interdiff de görülebilir
Commit’ler Git’in temel yapı taşlarıdır. Sadece commit kullanın
GitContext’in fixup, rebase, yeniden sıralama gibi işlemlerden geçmiş commit’leri nasıl takip ettiğini merak ediyorum
Bu yazının yazarıyım. Bana istediğinizi sorun
Ek olarak, kurallara aykırı mı bilmiyorum ama yakın zamanda yeni bir işe ihtiyacım oldu. Geliştirme araçları ve zor problemlerle uğraşmayı seviyorum. Bu yazı hoşunuza gittiyse, geliştirme ekibinizin üretkenliğini artırmak istiyorsanız ya da harika ve zaman zaman eklektik zevkimi deneyimleyip bundan keyif almak isterseniz profilimde e-posta adresim var
Ciddi konuşursam, özgeçmişin etkileyici. İyi ve hızlı şekilde yerleşmeni dilerim. Çok yakın zamandaki iş arama deneyimime göre mevcut işe alım piyasası ölümcül derecede hasta. Ne kadar kıdemli ve deneyimliysen, çılgın mülakat/İK uygulamaları ve açıklanmayan retler ruha o kadar daha çok acı veriyor
Bir arkadaşım aşağıdaki bağlantıları gönderdi
https://danluu.com/hiring-lemons/
https://danluu.com/programmer-moneyball/
https://danluu.com/algorithms-interviews/
Bol şans
Henüz değinilmeyen bir nokta da şu: diff soup yüzünden insanlar fixup commit’lerinin “gürültüsünü” ortadan kaldırmak için squash merge stratejisini tercih etmeye başlayabiliyor. Bu da baştaki iyi, atomik 3 commit’i de birlikte çöpe atıyor
interdiff inceleme stilinde baştaki 3 commit korunur; bunları tek tek mi yansıtacağınıza yoksa squash mı edeceğinize karar vermek tamamen commit’in kendisine ve gerçek atomikliğine bağlı olabilir
GitHub’ın acısını azaltmaya çalışan harici servisler de var ve stacked diff desteği sunuyorlar ama oldukça pahalı görünüyorlar. Özellikle self-hosted Gerrit ile karşılaştırınca daha da öyle. Bu araçlara da bakıp bakmadığınızı merak ediyorum
Eskiden MR serisi desenini oldukça kolay kurduğumu hatırlıyorum. 3 MR merge edilince, her MR için tek bir squash commit’i olan 3 merge commit oluşuyordu. MR’ın commit geçmişinden bağımsızdı
Ödünleşim, inceleme sırasında serinin ön tarafındaki branch değişirse serinin arka tarafındaki branch’e yeniden merge etmek gerekmesiydi
Yine de inceleme sırasında insanlar commit geçmişini istedikleri gibi düzenleyebiliyordu. Nasıl olsa squash edileceği için fark etmiyordu
Ancak bunu yapalı uzun zaman oldu ve bugünlerde GitHub’a bağlı yaşadığım için bunu pembe gözlüklerle hatırlıyor olabilirim
Review Board [https://www.reviewboard.org] üzerinde 2006'da zaten interdiff'i geliştirmişti. Aslında terimi ben uydurmuş ya da ona bağımsız olarak ulaşmış olabilirim
Hâlâ ürünün ve kod inceleme sürecinin en sevdiğim yanlarından biri; insanların GitHub gibi şeylere geçerken en çok özlediklerini söyledikleri özelliklerden biri olduğunu da sık sık duyuyorum
fix-it commit'leri bana hiçbir zaman düzgün bir alternatif gibi gelmedi
Birincisi, üstteki değişikliğin commit serisine nasıl yansıdığını göstermiyor
İkincisi, geçici de olsa commit grafiğini bozuyor ve incelemeyi zorlaştırıyor. İncelemeyi baştan beri takip eden biri, fix-it commit'inin düzelttiği kodu zaten okumuş olabilir; ama sonradan katılan biri, o kodun ne yapmaya çalıştığı veya yapısının nasıl olduğu konusunda kötü bir ilk izlenimle başlayabilir
Üçüncüsü, herkes Git ya da birden fazla commit'i yönetebilen sürüm kontrol sistemleri kullanmıyor. Oyun geliştirmede Perforce, çip üreticilerinde Keysight SOS gibi özel sürüm kontrol sistemlerinin kullanıldığı çok oluyor. Böyle yerlerde fix-it commit'inin kendisi bile bir seçenek değil
Doğru interdiff desteğine sahip bir kod inceleme sistemi varsa, bir incelemeci ilk yayımlanan inceleme isteğinden itibaren tüm güncellemeleri takip edip yalnızca değişenleri görebilir; başka bir incelemeci ise doğrudan en güncel tam değişikliğe girip fix-it serisiyle uğraşmak zorunda kalmaz. Üstelik bu, sürüm kontrol sisteminden bağımsız olarak mümkün olur
Doğru yapılırsa, birden çok commit'ten oluşan değişikliklerle de birlikte var olabilir
Örneğin küçük bir projeyi incelemeyi kolaylaştırmak için API handler'ı, frontend UI ve dokümantasyon olmak üzere birkaç commit'e böldüğünüzü; commit'lerin de birbirleriyle yakından ilişkili olduğu için tek bir inceleme isteği olarak gönderilmesinin uygun olduğuna karar verdiğinizi düşünebilirsiniz. Birbirleriyle daha az ilgili olsalardı, bağımlılık zinciri şeklinde birden çok inceleme isteği oluşturmak muhtemelen ideal olurdu
İnceleme geri bildirimine göre bu commit'lerden biri veya hepsi üzerinde bir dizi değişiklik yapabilirsiniz. İnsanlar güncellemeyi incelerken fix-it commit'lerini ve o değişikliği zihinlerinde karşılık gelen özgün değişiklikle eşlemek zorunda kalmadan, her bölümün nasıl evrildiğini görebilseler iyi olur
Bu yüzden interdiff gerçekten harika. Küçük commit'ler çok olsun ya da büyük commit'ler olsun, tek commit'lik inceleme isteği olsun ya da çok commit'li olsun, daha fazla insanın kullanması gerekiyor
2013 civarında Glasgow Haskell Compiler'ı şakayla karışık “bug raporlayıcıda .patch dosyası okuma” yönteminden Phabricator kullanımına taşıdık. Bunun birkaç nedeni vardı ama o dönemde sebep stacked diff değildi. Çünkü GitHub inceleme için o kadar kötüydü ki side-by-side diff bile yoktu. Sırf bu nedenle bile tamamen konu dışıydı; başka nedenler de vardı
Ama bir ekibi kod inceleme aracına taşıdığım ilk sefer bu değildi. 2009'daki ilk işimde, Houston'da bir odada toplanmış çok küçük ve sıkı bağlı bir mühendis ekibindeydim; kodumu başkalarına inceletmenin ve onların yazdıklarını okumanın kod tabanını daha iyi anlamamı sağlayacağını düşünmüştüm
Bu yüzden işe girdikten sonraki ilk birkaç ay boyunca yöneticimin başının etini yiyip ReviewBoard kurdurttum; herkes de gerçekten sevdi
Yani RB'ye teşekkürler demek istiyorum. Hâlâ ara sıra güzel bir anı olarak aklıma gelir. Onun sayesinde kod inceleme neredeyse ilk günden itibaren kariyerimin çok büyük bir parçası oldu ve bugün hâlâ daha fazlasını yapabileceğimizi düşünüyorum
Genel olarak kod incelemeyi merkeze alan ve rebase odaklı Gerrit gibi sistemlerde kodun çok daha kolay incelendiğini hissettim
En iyi yanlarından biri, birden çok patch'i üst üste koymayı yerleşik olarak desteklemesi; bu da insanların daha küçük ve incelenmesi daha kolay patch'ler hazırlamasını sağlıyor
GitHub'ın kod incelemesi kötü bir sonradan eklenmiş özellik gibi hissettiriyor: forum thread'i gibi görünen, yer israfı yapan UI ve rebase'in ötesini takip edememesi gibi şeyler
Bu tür “büyük resim” niteliğinde ama yine de teknik tartışmalar çok sık issue tracker'da yaşanıyor. Ama nereye konması gerektiğini de bilmiyorum. Bu stack fazla geçici ve bir sonraki push'ta tamamen değişebilir
Bu stratejiyi savunanları birkaç kez duydum ama ikna olmadım. Üzerinde çalıştığım projelerin çoğunda feature branch tek bir commit’e squash edilip merge ediliyor, branch geçmişi de siliniyor
Yazarın anlattığı türden bir durum olsaydı, refactoring’i, yeni API’yi ve güncellemeyi basitçe 3 commit olarak oluştururdum
Benim için kesinlikle iyi bir pratik olan şey uzun ömürlü branch’lerden kaçınmak. Bu tür çok aşamalı commit senaryoları genelde oradan çıkıyor. Yani birinin bir özellik üzerinde günlerce, hatta bazen haftalarca çalışıp sonra hepsini tek seferde ortaya atmak istemesi durumu
Ben her gün, hatta günde birkaç kez main’e taşımayı çok daha fazla tercih ederim. Bunu yapmanın bir yolu, devam eden commit’lerin de sorunsuzca dahil edilebilmesi için feature flag kullanmak. Sistem düzgün yapılandırılmışsa, geliştirme ve staging ortamlarında açıp test edebilme avantajı da var. Blue/green dağıtıma da bir adım mesafede
Büyük commit’leri daha kolay review etmenin bir yolunu istemiyorum. Ekibi küçük değişiklikleri erken ve sık commit etmeye zorlamak istiyorum. Herkesin buna katılmadığını anlıyorum
Bunun nedenlerinden biri, GitHub PR’larından farklı olarak stack’in yalnızca bir kısmının uygulanabilmesi. Yazıdaki örnek üzerinden bakarsak, “küçük refactoring” diff’i hazırsa, “yeni API” ve “API kullanıcılarının migration’ı” diff’lerini birlikte almadan önce onu içeri alabilirsiniz
Branch yerine commit merkezli düşününce, uzun ömürlü branch’lere kıyasla toplam commit’lerin daha küçük olması gibi bir etkisi oluyor
Bunu yapınca
git blamevegit bisectfiilen işe yaramaz hale geliyorBir regresyon varsa
git bisectbunu tek bir patch’e kadar daraltabilir. Bu yüzden belirli bir özellik için git geçmişinde 8000 satırlık tek bir patch yerine 160 satırlık 50 patch olması daha iyidirBir kod satırı şüpheli görünüyorsa,
git blameya da art arda çalıştırılangit blamekomutlarının beni 8000 satırlık bir commit’e değil, ayrıntılı commit mesajı olan 160 satırlık bir commit’e götürmesini isterimOrijinal commit’lerin sırası da korunmalı. Yıllar sonra yalnızca tek tek commit mesajlarını sırayla okumak bile özgün tasarımı anlamakta çok yardımcı olabilir. Elbette orijinal patch set’i bağımlılık sırasına göre düzenlenmiş olmalı, her aşamada derlenebilmeli vb.; yani özellik implementasyonunun üstünde ayrı bir geliştirme aşaması gerekir. Kod mantıksal adımlar halinde sunulmalı
İlginç. İş yerinde PR’ları yazarın commit’leri kullandığı şekilde kullanıyoruz ve en sonunda gerçekten squash-and-merge yapıyoruz
Ama bu yöntemde öndeki PR’ı her değiştirdiğinizde arkadaki PR’ları rebase etmeniz gerekiyor. Oldukça zahmetli; “force-push yasak” kuralına da takılıyor, mühendislerin öğrenmesi uzun sürüyor ve GitHub UI’daki mevcut code review yorumlarını bozma eğiliminde. Yine de iki üç PR için fena çalışmıyor
Bizim akışımızda commit’ler iş biriminden çok kayıt noktalarına benziyor
stash’i de çok kullanıyoruz, Emacs’in undo-tree-mode’unu da. Böylece kaynak kod geçmişini izlemek için dört yöntem oluyor; kulağa tekrar gibi geliyor ama pratikte gayet iyi işliyor
Git’te bunu yönetmenin kullanılabilirliği oldukça kötü. Phabricator daha iyiydi ama yine de gereksiz yere zordu. Belki yeni bir sürüm kontrol aracı commit ve branch’ten daha üst düzey kavramları birinci sınıf olarak destekleyebilir; belki de bu onu kullanmayı daha da kötü hale getirir
Git’ten daha iyi bir araç olsa iyi olur muydu? Elbette ben de daha az kusurlu ve daha kullanıcı dostu bir araç isterim. Ama geçici bir çözüm olarak commit’leri commit gibi ele almaya ne dersiniz? İşi kendi kendinize zorlaştırmayın
GitHub’ın insanlara PR gözlüğü taktırmadaki sorumluluğunun ne kadar büyük olduğunu merak ediyorum
jj,sapling,mercuriala bakmanız iyi olurBlog yazısı güzeldi; code review’un nasıl çalışması gerektiğini gerçekten düşünen insanları görmek sevindirici
Farklı artı ve eksileri olan dört code review sistemini epey kullandım: Critique(Google içi), Gerrit(Google’da kullandık ama dışarıdakiyle aynı), GitHub ve kendi yaptığım CodeApprove
Critique açık ara en iyisiydi, ama bu Google’ın monorepo’suna, kendi sürüm kontrol sistemine ve custom lint/test araçlarına mükemmel şekilde uyarlandığı için mümkündü. CodeApprove, bu avantajları mümkün olduğunca GitHub’a taşımak için tasarlandı ama pratikte ona yaklaşmak zor
Gerrit reviewer deneyimi açısından ikinci en iyisiydi, ama author olarak her zaman hoşlanmadım. Doğru yöntemden çok yanlış yapma yolu varmış gibi görünüyordu ve UI da özellikle güzel değil
GitHub author için çok dostça ve bizim düşündüğümüz şekilde çalışıyor. Kod yazıyorsunuz, feedback alıyorsunuz, daha fazla kod yazıyorsunuz. PR sonunda squash and merge yaparsanız yazarın bahsettiği geçmiş sorunu olmaz
Ama reviewer’lar ya da ekip için pek dostça değil. Artımlı diff vurgulanmıyor, diff ve konuşma farklı sekmelerde, force push ve rebase geçmişi yok ediyor. Yorumlar “outdated” diye kayboluyor, diff penceresi dışındaki dosyalara yorum bırakamıyorsunuz, büyük dosyalar varsayılan olarak gizleniyor ve daha pek çok sorun var. GitHub’ın bunu pek umursamadığı açık; belki de benim bilmediğim bir şey biliyorlardır
Sonuçta en sinir bozucu olan, birçok ekibin sürüm kontrol platformuna gömülü code review aracını olduğu gibi kabul etmesi. Dizüstü bilgisayarda varsayılan gelen IDE’yi aynen kullanmak gibi. Bugün çok daha iyi seçenekler var. CodeApprove dışında sevdiğim araçlar GitContext, Reviewable, Graphite; ayrıca en az altı tane daha harika seçenek sayabilirim. Varsayılanı kabul etmemek gerekir
Reviewer’lar ve maintainer’lar contributor’lardan çok daha kıt. İş akışı ve UI’ın review throughput’unu optimize etmesi gerektiğini düşünüyorum
Bahsedilen diğer üç aracı hiç kullanmadım; bu yüzden iddiam genel bir değerlendirme