- Kiểu review PR của GitHub vốn rất dễ tiếp tục chồng thêm các commit sửa lên trên thay đổi ban đầu, khiến quá trình review và lịch sử code cuối cùng bị trộn lẫn vào nhau
- Một chuỗi patch tốt phải có thể được đọc theo thứ tự dưới dạng các commit tách biệt về mặt logic, như refactor, thêm API mới, rồi migrate người dùng hiện có sang API đó
- Khi các commit như “fix review” hay
"minor" chồng lên nhau, ý đồ thay đổi sẽ trở nên mờ nhạt, và chi phí tìm nguyên nhân sự cố bằng git blame và git bisect sẽ tăng lên
- Review interdiff phát hành phiên bản mới của chuỗi patch và dùng
git range-diff main..v1 main..v2 để so sánh khác biệt theo từng commit
- Reviewer không cần đọc lại toàn bộ 500 dòng mà chỉ cần kiểm tra phần tăng thêm 50 dòng, còn tác giả có thể giữ nguyên cấu trúc commit cuối cùng mà không cần các commit nhiễu kiểu “address review”
Công cụ code review và điểm khởi đầu
- Gerrit Code Review là công cụ code review mã nguồn mở hoạt động cùng kho Git, hỗ trợ luồng gửi patch để người khác xem xét mã và để lại bình luận
- Cách làm code review rất đa dạng, không chỉ có Gerrit, GitHub hay Phabricator mà còn có kiểu tải file
.patch lên bug tracker, git send-email, hoặc dẫn dắt việc triển khai sau những cuộc thảo luận dài với đồng nghiệp
- Ngôn ngữ lập trình Go dùng Gerrit, KDE và LLVM từng dùng Phabricator, còn Jujutsu cũng từng cân nhắc dùng Gerrit
- Linux kernel sử dụng workflow dựa trên email theo cách riêng, còn nhiều dự án chọn GitHub vì dễ dùng và chi phí bắt đầu thấp
- Vấn đề cốt lõi là các công cụ không cung cấp cùng một mô hình review, và trải nghiệm mặc định của GitHub không phải lúc nào cũng đủ tốt
Điều kiện của một chuỗi patch tốt
- Một chuỗi patch lý tưởng là dạng gửi một thay đổi lớn nhưng được chia thành các bước logic
- Trước tiên thực hiện dọn dẹp mã được phát hiện ngẫu nhiên
- Tiếp theo thêm API mới
- Cuối cùng migrate những chỗ đang dùng API cũ sang API mới
- Mỗi patch nên có lý do tồn tại độc lập, và khi áp dụng theo thứ tự thì phải thể hiện được cách mã tiến hóa từng bước
- Cấu trúc như vậy không chỉ có lợi cho tác giả và reviewer, mà còn hữu ích cho người bảo trì sau này khi phải truy tìm lý do thay đổi bằng
blame hoặc bisect
- Kích thước patch thực tế có thể khác nhau tùy tình huống
- Có thể không có refactor
- Có thể chỉ 100 dòng
- Hoặc có thể là 500 dòng nếu như thay đổi API cốt lõi buộc phải sửa mọi call site
“Diff soup” kiểu GitHub tạo ra lịch sử bị pha trộn
- Khi phản ánh bình luận review, GitHub khuyến khích khá rõ ràng, cả công khai lẫn ngầm định, luồng thêm commit mới lên trên các commit gốc
- Trong ví dụ, sau 3 commit ban đầu lần lượt chồng thêm các commit phản hồi review của Alice và Bob, thêm test, rồi thay đổi triển khai
"minor"
- Vì đồ thị commit chỉ giữ lại quan hệ cha-con, nên không thể hiện được commit sửa review đang sửa vấn đề nào của commit gốc nào
- Nếu một commit “fix review” cùng lúc chạm vào nhiều commit gốc thì mô hình khái niệm còn phức tạp hơn
- Có thể ép để một commit sửa chỉ phản ánh đúng một nhận xét, nhưng đổi lại số lượng commit fixup sẽ tăng lên
- Dạng mà thay đổi lớn và các chỉnh sửa review bị trộn chung trong một nồi như vậy được gọi là “diff soup”
Tín hiệu của git blame và git bisect trở nên mờ hơn
git blame hoạt động theo từng dòng, nên chỉ cần một commit phản hồi review sửa một phần nhỏ của dòng hiện có thì nguồn gốc của dòng đó có thể hiện thành commit như "fix alice review"- Muốn tìm lý do thay đổi thực sự thì phải lần ngược lại commit gốc và các commit sửa review, tức là cần đến khảo cổ commit
- Trong
git bisect cũng vậy, sẽ khó biết commit "minor" có phải nguyên nhân thật hay chỉ là lỗi vốn đã tồn tại từ lúc API mới được thêm vào nhưng lộ ra muộn hơn
- Nếu API mới mang lại cải thiện hiệu năng lớn thì việc revert có thể không mong muốn hoặc thậm chí không khả thi
- Nếu regression xuất hiện sau vài tuần, thì trong tình huống khẩn cấp như cuộc họp P1, chỉ nhìn vào message commit
"minor" cũng khiến phát sinh chi phí điều tra thêm
- Ngay cả trong ví dụ chỉ qua lại review hai lần, lịch sử đã trở nên phức tạp; với các dự án mã nguồn mở nơi review lặp lại nhiều vòng, vấn đề còn lớn hơn
Cách UX của GitHub củng cố luồng này
- Trên GitHub, nếu muốn review tăng dần thì về thực tế cách dễ nhất là thêm commit mới lên trên
- Với reviewer, xem patch
"fix alice review" có +/- 10 dòng sẽ đỡ nặng hơn nhiều so với đọc lại 500 dòng của API mới
- Tab diff của PR mặc định gộp tất cả commit thành một diff duy nhất
- Nó gần giống với việc xem
git diff master..foo-branch
- Điều này không khớp lắm với cách mọi người viết và đọc thay đổi
- UX của GitHub bám rất chặt vào mô hình branch có tên, nên khó xem khác biệt giữa các đơn vị ngoài branch
- Sau khi force push, vẫn có thể xem diff bằng nút “Compare” ở dòng “Force Push” của PR
- Nhưng nó chỉ hiển thị toàn bộ diff giữa head branch trước đó và head branch hiện tại
- Muốn so sánh chi tiết hơn thì phải tự tìm ID commit rồi đưa vào URL
Review interdiff và git range-diff
- Review interdiff không thêm các commit sửa lên trên 3 commit gốc, mà phát hành một phiên bản mới cũng gồm đúng 3 commit đó
- Ví dụ, nếu cần phản ánh bình luận của Bob vào chuỗi patch
v1, thì chỉnh sửa commit thứ nhất và thứ ba để tạo thành chuỗi v2
- Sau đó dù tiếp tục phản ánh bình luận của Alice, thêm test và thay đổi
"minor" theo cách tương tự, kết quả cuối cùng vẫn chỉ còn 3 commit
- Nếu các commit A, B, C của
v1 tiến hóa thành X, Y, Z của v2, thì git range-diff sẽ hiển thị khác biệt giữa các commit theo từng vị trí
git range-diff \
main..v1 \
main..v2
- Lệnh này hiển thị diff theo cặp giữa 3 commit của branch
v1 và 3 commit của branch v2
- Commit thứ nhất là
diff(A, X)
- Commit thứ hai là
diff(B, Y)
- Commit thứ ba là
diff(C, Z)
Hiệu quả thực tế của cách interdiff
- Khác với GitHub thường hiển thị diff toàn branch như
diff(main, C) hoặc diff(main, Z), interdiff hiển thị khác biệt giữa các phiên bản commit
- Reviewer không cần đọc lại thay đổi API dài 500 dòng đã xem trước đó, mà chỉ cần kiểm tra 50 dòng đã được sửa
- Tác giả cũng không cần chất 30 commit nhiễu kiểu “address review” vào lịch sử
git blame có khả năng cao hơn sẽ nối dòng mã về commit thay đổi gốc với ít nhiễu hơngit bisect có khả năng cao hơn sẽ chỉ ra các đơn vị có ý nghĩa hơn như commit thêm API mới khi tìm regression- Kết quả là tỷ lệ tín hiệu trên nhiễu của các công cụ chẩn đoán mặc định được cải thiện
Ghi chú về chiến lược merge và rebase
- Phần giải thích về chiến lược merge patch hiện vẫn được để dưới dạng TODO
- Quan điểm ở đây là
git rebase vẫn ổn miễn là đó không phải nhánh công khai nơi người khác được kỳ vọng sẽ tiếp tục chồng commit lên trên
- Hệ thống review interdiff khuyến khích các patch nhỏ hơn và có thể merge nhanh hơn
- Không cần đợi đến khi cả 5 commit đều sẵn sàng
- Nếu 3 commit đầu đã ổn còn 2 commit sau cần thêm thời gian thì có thể chỉ merge 3 commit đó
- Mục tiêu là khiến mọi người làm việc dựa trên nhánh main thay vì các nhánh dài hạn, qua đó giảm tình trạng nhiều nhánh remote quấn vào nhau rồi merge lẫn nhau
- Cũng có những dự án như Linux kernel, nơi nhánh công khai được merge với kho khác hoặc được công khai rõ ràng trên nền các nhánh khác
- Các nhà phát triển Linux có thể dùng
git rebase khi tạo và tinh chỉnh chuỗi patch
- Nhưng sau khi đã tạo nhánh công khai, vì nó có thể được merge vào tree của ai đó nên họ không force push mà tạo nhánh mới có thêm commit
1 bình luận
Ý kiến trên Hacker News
Trên GitHub tôi nhìn chung đang dùng luồng như thế này; nhược điểm là tôi phải làm nhiều việc hơn và nó không trực quan với cộng tác viên
Dù vậy, nó vẫn giữ được ưu điểm là reviewer có thể xem phần chênh lệch chỉ phản ánh feedback của mình, và không làm hỏng
git blamehaygit bisectKhi phản hồi feedback review, tôi commit bằng
git commit --fixup, sau đó push rồi để lại hash của commit fixup trong phần trả lời comment reviewKhi PR đã được approve và sắp merge, tôi chạy
git rebase --interactive origin/main --autosquashđể gộp các commit fixup vào commit gốc, cuối cùnggit push --force-with-leaserồi mergeCần cẩn thận với force push trước khi review kết thúc. Làm vậy sẽ khiến reviewer không xem được những nội dung được thêm sau lần review cuối cùng
Tôi phụ thuộc khá nhiều vào autocomplete trong terminal nên chỉ cần gõ cỡ
git relà đi tới được cả lệnh dài, nhưng cách này hơi thô; sẽ tốt hơn nếu có công cụ hỗ trợ và khuyến khích luồng này. Dù vậy nếu bị buộc trong GitHub thì như thế này cũng ổnTuy nhiên trên thực tế nó thường chỉ được dùng trong các đội engineering có kỷ luật, chủ động học một số tính năng nâng cao của git
~/.gitconfig[rebase] autosquash = trueKhi đó về sau chỉ cần
git rebase -i origin/mainlà các commit fixup/squash sẽ tự động được sắp xếp lại. Đây là một thiết lập nhỏ nhưng cải thiện workflow rất nhiềuĐây là lĩnh vực tương tự như việc cần
--update-refskhi rebase một chuỗi commit có nhiều branch nằm phía trên. Tại sao con người phải theo dõi quan hệ đồ thị, tự chèn commit và di chuyển branch? Máy tính xử lý đồ thị rất giỏi, cứ giao cho nóCũng có
git absorb, nhưng nó không vững chắc bằng triển khai của Sapling[1]Vấn đề thật sự không phải là có rebase tương tác hay không, mà nói chung là trải nghiệm người dùng của chính công cụ review và vòng lặp mà nó dẫn dắt. Ví dụ commit fixup không giải quyết được chuyện GitHub hiển thị chênh lệch giữa các mốc tham chiếu, và nếu mốc tham chiếu lớn thì có thể phá hỏng hoàn toàn việc review. Chẳng hạn khi rebase lên trên 10 commit mới
Tôi cũng có bất mãn với UX của chính Git, nhưng bài gốc chủ yếu giống lời phàn nàn về GitHub hơn
[1] Ví dụ cho thấy khác biệt giữa hai thuật toán nền nằm trong issue GitHub này: https://github.com/martinvonz/jj/issues/170
Tôi đồng ý 100% rằng cách này là lý tưởng. Cách của GitHub thật sự tệ hại, và bi kịch là quá nhiều người đã xem nó là chuyện hiển nhiên
Trước đây chúng tôi làm như vậy bằng Phabricator, và nhờ macro dòng lệnh cập nhật tất cả review cùng lúc nên có thể làm thủ công ở mức nào đó. Dù vậy có một UI rõ ràng thì vẫn tốt hơn
Đúng vậy. Đây chính là thứ tôi hình dung trong đầu như một phong cách code review thực sự, chứ không phải cách GitHub đang làm. Rất vui khi biết nó có tên gọi
Thêm nữa, sẽ tốt nếu hệ thống review có thể đẩy các patch đã sẵn sàng “ra khỏi” review. Một bug fix nhỏ được tạo ra trong khi làm một tính năng lớn nên là một patch nhỏ, độc lập, và có khả năng nhanh chóng đạt đồng thuận với reviewer
Khi đó tôi muốn tách patch đó ra khỏi toàn bộ series, cherry-pick vào main, rồi rebase review lên HEAD mới. Hoặc cũng có thể rebase series patch mới nhất lên main nhưng sắp xếp lại patch đã được đồng thuận lên đầu, sau đó fast-forward main tới patch đó
Về bản chất, ý là thu hẹp phạm vi review vào “phần vẫn đang được thảo luận”, đồng thời để bug fix được merge ngay khi sẵn sàng
Phản biện sẽ là “cứ tạo review/PR riêng đi”, nhưng như vậy sẽ sinh ra độ phức tạp kiểu patchset A phụ thuộc vào patchset B, rồi sau khi B được merge thì lại phụ thuộc vào main
Những ý tưởng UX thông minh như Attention Set cũng góp phần vào việc này. Về cơ bản, nó biểu thị “ai là người cần hành động tiếp theo?”, vận hành giống như game theo lượt. Nếu tôi vừa review xong, attention set của patch đó không còn có tôi nữa mà là tác giả
Vì vậy trong UI Gerrit, mục đó sẽ tụt xuống phía dưới trong hàng đợi của tôi. Ở đầu hàng đợi là các mục mà tôi nằm trong attention set. Công việc tự nhiên được gom lại theo cách như vậy
Tôi chưa đề cập hết các vấn đề nhỏ nhặt khó chịu khác trong UX của GitHub, nhưng ngay cả danh sách pull request cũng tệ hơn các lựa chọn thay thế. Không thể biết mục nào đang ở trạng thái nào, cuối cùng vẫn phải đọc toàn bộ
Nếu là x ở dưới cùng thì có thể submit ngay và tiếp tục với phần còn lại
Việc thấy một cách tiếp cận mới với code review lúc nào cũng thú vị. GitHub cũng có ưu điểm, nhưng còn xa mới hoàn hảo
Với kịch bản được nêu, có thể cân nhắc tách ba patch thành các pull request riêng phụ thuộc lẫn nhau. GitHub không hỗ trợ sẵn, nhưng một công cụ code review phù hợp nên có thể quản lý phụ thuộc trong khi vẫn giữ pull request nhỏ. Ví dụ, patch 3 có thể phụ thuộc vào patch 2, và patch 2 phụ thuộc vào patch 1
Nhờ việc theo dõi phụ thuộc mà công cụ cung cấp, có thể đảm bảo tất cả được triển khai cùng nhau nếu cần
Review từng patch riêng sẽ giúp feedback rõ ràng hơn và phản hồi cũng dễ hơn. Cũng có thể squash các commit trong pull request để tạo lịch sử commit gọn gàng, phản ánh chính xác từng thay đổi riêng lẻ. Xa hơn nữa, nếu công cụ phù hợp thì có thể dùng AI để tóm tắt pull request và review, đơn giản hóa việc viết commit message chính xác mà không cần làm thủ công
Một công cụ code review tốt không nên bị chi phối bởi các thao tác git như rebase, merge, force push. Dù phía sau có diễn ra các thao tác git phức tạp nào, reviewer chỉ nên thấy những thay đổi kể từ lần review cuối. Như vậy họ có thể tập trung vào nội dung mới mà không phải review lại diff lớn, và lịch sử review được giữ sạch sẽ, tách biệt khỏi lịch sử commit
Tôi tò mò liệu cách chia pull request như vậy và theo dõi phụ thuộc lẫn nhau có đáp ứng được yêu cầu không
Nói đúng ra, đây là workflow mà git được thiết kế để hỗ trợ, và là trọng tâm của lời phê bình cũ đã tồn tại kể từ khi GitHub có pull request
Tôi tò mò vì sao ý tưởng này, xem lại lịch thì, đã bị coi thường suốt hơn 15 năm như cách làm của “mấy ông già mailing list hết thời”, mà giờ mới leo lên được các nhóm văn hóa phát triển đang thịnh hành
Tôi từng nghĩ rằng vì nhiều lập trình viên phần mềm đã nhất quyết từ chối học cho đúng một trong những công cụ họ dùng nhiều nhất, nên họ đang cố tình chọn workflow tệ. Vì vậy khi chuyển công ty sang git, tôi đã chọn và xây dựng những công cụ và luồng làm việc như thế. Dù sao thì mua IBM cũng đâu khiến bạn bị sa thải
Khi contributor bên ngoài dùng fork của repository, ba PR sẽ lần lượt chứa A, A+B, A+B+C. Khi đó trong hai PR cuối, bạn lại phải thấy trong diff cả phần code đã review ở PR khác, khiến việc review khó hơn
Mỗi commit đều có thể review độc lập. Cũng có thể theo dõi phụ thuộc giữa các commit. Việc không tích hợp AI sẵn cũng là một điểm tốt. Khác với PR, giữa các commit còn xem được interdiff
Commit là vật liệu cơ bản của Git. Cứ dùng commit là được
Tôi tò mò GitContext theo dõi các commit đã trải qua những thao tác như fixup, rebase, sắp xếp lại như thế nào
Tôi là người viết bài này. Hỏi gì cũng được
Thêm nữa, tôi không biết có trái quy định không, nhưng gần đây tôi cần một công việc mới. Tôi thích làm việc với công cụ phát triển và các vấn đề khó. Nếu bạn thích bài này, hoặc muốn tăng năng suất của đội phát triển, hoặc muốn trải nghiệm và tận hưởng gu tuyệt vời, đôi khi hơi chiết trung của tôi, email có trong hồ sơ
Nói nghiêm túc thì CV của bạn rất ấn tượng. Chúc bạn sớm tìm được chỗ phù hợp, và tìm được thật nhanh. Theo trải nghiệm tìm việc rất gần đây của tôi, thị trường tuyển dụng hiện tại đang bệnh nặng. Bạn càng senior và càng nhiều kinh nghiệm, các thực hành phỏng vấn/HR điên rồ và những lần bị từ chối không lời giải thích càng làm tổn thương tinh thần hơn
Một người bạn đã gửi cho tôi các link dưới đây
https://danluu.com/hiring-lemons/
https://danluu.com/programmer-moneyball/
https://danluu.com/algorithms-interviews/
Chúc may mắn
Một điểm chưa được đề cập là do diff soup, mọi người có thể sẽ thích chiến lược squash merge để loại bỏ “nhiễu” từ các commit fixup. Khi đó họ cũng vứt luôn cả 3 commit nguyên tử tốt ban đầu
Với phong cách review interdiff, 3 commit ban đầu được giữ lại, và việc áp dụng riêng lẻ hay squash hoàn toàn có thể được quyết định dựa trên chính các commit đó và tính nguyên tử thực sự của chúng
Cũng có các dịch vụ bên ngoài cố giảm nỗi đau khi dùng GitHub và cung cấp hỗ trợ stacked diff, nhưng trông có vẻ khá đắt, đặc biệt khi so với Gerrit self-hosted. Tôi tò mò liệu bạn đã xem qua các công cụ như vậy chưa
Tôi nhớ trước đây từng cấu hình pattern chuỗi MR khá dễ. Khi merge 3 MR, sẽ tạo ra 3 merge commit, mỗi MR có một squash commit duy nhất. Việc này không phụ thuộc vào lịch sử commit của MR
Điểm đánh đổi là trong lúc review, nếu branch ở phần đầu của chuỗi thay đổi thì phải merge lại vào branch phía sau trong chuỗi
Dù vậy, trong quá trình review, mọi người có thể xử lý lịch sử commit theo ý muốn. Dù sao cuối cùng cũng bị squash nên không sao
Tuy nhiên đã lâu rồi tôi không làm vậy, và dạo này bị trói vào GitHub nên có thể tôi đang nhìn lại bằng ký ức màu hồng
Review Board [https://www.reviewboard.org] đã tạo ra interdiff từ tận năm 2006. Thật ra có thể tôi đã đặt ra thuật ngữ đó, hoặc tự mình đi đến nó một cách độc lập
Đến nay đây vẫn là phần tôi thích nhất trong sản phẩm và trong quy trình review code, và là một trong những tính năng mà tôi thường nghe mọi người nói là nhớ nhất khi họ chuyển sang những thứ như GitHub
Tôi chưa bao giờ cảm thấy các commit fix-it là một phương án thay thế đúng nghĩa
Thứ nhất, chúng không cho biết thay đổi cấp trên đã được phản ánh vào chuỗi commit như thế nào
Thứ hai, dù chỉ tạm thời, chúng cũng làm rối đồ thị commit và khiến việc review khó hơn. Người đã theo dõi quá trình review có thể đã đọc phần code mà commit fix-it sửa, nhưng người mới tham gia có thể bắt đầu với ấn tượng ban đầu không tốt về việc đoạn code đó định làm gì hoặc cấu trúc của nó ra sao
Thứ ba, không phải ai cũng dùng Git hay một hệ thống quản lý phiên bản có thể xử lý nhiều commit. Trong phát triển game, người ta thường dùng Perforce; ở các hãng sản xuất chip, cũng có nhiều trường hợp dùng các hệ thống quản lý phiên bản chuyên biệt như Keysight SOS. Ở những nơi như vậy, bản thân commit fix-it không phải là một lựa chọn
Với một hệ thống review code hỗ trợ interdiff đúng nghĩa, một reviewer có thể theo dõi từ yêu cầu review được công khai lần đầu qua mọi bản cập nhật và chỉ xem những gì đã thay đổi, trong khi một reviewer khác có thể đi thẳng vào toàn bộ thay đổi mới nhất mà không cần bận tâm đến chuỗi fix-it. Và điều này có thể thực hiện bất kể hệ thống quản lý phiên bản nào
Nếu được làm đúng, nó cũng có thể cùng tồn tại với các thay đổi gồm nhiều commit
Ví dụ, bạn có thể chia một dự án nhỏ thành nhiều commit như API handler, UI frontend và tài liệu để hỗ trợ review, và vì các commit liên quan chặt chẽ với nhau nên cho rằng đưa chúng lên trong một yêu cầu review là phù hợp. Nếu chúng ít liên quan với nhau, lý tưởng hơn có lẽ là tạo nhiều yêu cầu review theo chuỗi phụ thuộc
Dựa trên phản hồi review, bạn có thể tạo một loạt thay đổi cho một hoặc tất cả các commit đó. Khi mọi người review bản cập nhật, sẽ tốt hơn nếu họ có thể thấy từng phần đã tiến hóa ra sao, thay vì phải tự ánh xạ trong đầu các commit fix-it và những thay đổi đó với thay đổi gốc tương ứng
Vì vậy interdiff thực sự rất tuyệt. Dù là nhiều commit nhỏ hay một commit lớn, yêu cầu review một commit hay nhiều commit, nhiều người hơn nên dùng nó
Khoảng năm 2013, Glasgow Haskell Compiler đã chuyển từ kiểu đùa là “đọc file .patch trong bug reporter” sang dùng Phabricator. Có vài lý do, nhưng lúc đó không phải vì stacked diff. Mà là vì GitHub quá tệ cho review, đến mức thậm chí còn không có side-by-side diff. Chỉ riêng lý do đó thôi cũng đã hoàn toàn không đáng xét, và còn có những lý do khác nữa
Nhưng đó không phải lần đầu tôi chuyển một nhóm sang công cụ review code. Công việc đầu tiên của tôi năm 2009 là trong một nhóm kỹ sư rất nhỏ và gắn bó, cùng ngồi trong một căn phòng ở Houston, và tôi nghĩ nếu người khác review code của mình rồi tôi đọc những gì họ viết, tôi sẽ hiểu codebase tốt hơn
Vì vậy trong vài tháng đầu sau khi vào làm, tôi đã năn nỉ quản lý cài ReviewBoard, và mọi người thật sự rất thích nó
Nên ý tôi là cảm ơn RB. Đến giờ thỉnh thoảng tôi vẫn nhớ về nó như một kỷ niệm đẹp. Nhờ nó, code review đã trở thành một phần rất lớn trong sự nghiệp của tôi gần như ngay từ ngày đầu, và tôi vẫn nghĩ mình có thể làm nhiều hơn nữa
Nhìn chung, tôi cảm thấy các hệ thống đặt code review lên trước và xoay quanh rebase như Gerrit khiến code dễ review hơn nhiều
Một trong những điểm hay nhất là chúng hỗ trợ sẵn việc xếp chồng nhiều patch, khiến mọi người tạo ra các patch nhỏ hơn và dễ review hơn
Code review của GitHub giống như một tính năng thêm vào sau khá tệ. UI lãng phí không gian trông như một thread diễn đàn, không thể theo dõi vượt qua rebase, v.v.
Những thảo luận vừa mang tính “bức tranh lớn” vừa kỹ thuật như vậy quá thường xuyên diễn ra trong issue tracker. Nhưng tôi cũng không biết nên đặt chúng ở đâu. Stack này quá tạm thời và có thể thay đổi hoàn toàn ở lần push tiếp theo
Tôi đã nghe người ta ủng hộ chiến lược này vài lần, nhưng vẫn chưa bị thuyết phục. Hầu hết các dự án tôi làm đều squash feature branch thành một commit duy nhất rồi merge, và xóa lịch sử của branch
Nếu gặp trường hợp như tác giả mô tả, tôi sẽ chỉ tạo 3 commit: refactor, API mới, và cập nhật
Điều chắc chắn là một thực hành tốt với tôi là tránh branch sống lâu. Kịch bản commit nhiều giai đoạn như thế này thường phát sinh từ đó. Tức là tình huống ai đó làm một tính năng trong vài ngày, thậm chí vài tuần, rồi muốn ném tất cả vào một lần
Tôi thích chuyển sang main hằng ngày, hoặc thậm chí nhiều lần trong ngày, hơn nhiều. Một cách để làm việc này là dùng feature flag để các commit đang làm dở vẫn có thể được đưa vào mà không gây vấn đề. Nếu hệ thống được cấu hình đúng, còn có lợi thế là có thể bật lên để kiểm thử trong môi trường dev/staging. Nó cũng chỉ cách triển khai blue/green một bước
Tôi không muốn một cách để review các commit lớn dễ hơn. Tôi muốn buộc đội commit các thay đổi nhỏ sớm và thường xuyên. Tôi hiểu là không phải ai cũng đồng ý
Một lý do là, khác với PR của GitHub, có thể đưa vào chỉ một phần của stack. Nhìn theo ví dụ trong bài, nếu diff “refactor nhỏ” đã sẵn sàng, có thể đưa nó vào trước mà không cần đưa kèm diff “API mới” và “di chuyển người dùng API”
Nếu đặt trọng tâm vào commit thay vì branch, nó có tác dụng làm tổng thể các commit nhỏ hơn so với branch sống lâu
Làm vậy khiến
git blamevàgit bisectgần như vô dụngNếu có regression,
git bisectcó thể thu hẹp đến một patch duy nhất. Vì vậy, với một tính năng cụ thể, có 50 patch mỗi patch 160 dòng trong lịch sử git tốt hơn là một patch 8000 dòngNếu một dòng code nào đó trông đáng ngờ, tôi muốn
git blamehoặc các lệnhgit blameliên tiếp dẫn tôi đến một commit 160 dòng với thông điệp commit chi tiết, chứ không phải một commit 8000 dòngCũng cần giữ nguyên thứ tự của các commit ban đầu. Vài năm sau, chỉ cần đọc các thông điệp commit riêng lẻ theo thứ tự cũng có thể rất hữu ích để hiểu thiết kế ban đầu. Tất nhiên patchset ban đầu phải được cấu trúc theo thứ tự phụ thuộc, phải biên dịch được ở từng bước, v.v., tức là cần một giai đoạn phát triển riêng bên trên việc triển khai tính năng. Code nên được trình bày theo các bước logic
Thú vị. Ở chỗ làm, chúng tôi dùng PR theo cách tác giả dùng commit, và cuối cùng thực sự squash-and-merge
Nhưng cách này đòi hỏi phải rebase các PR phía sau mỗi khi thay đổi PR phía trước. Khá phiền, vướng cả quy tắc “cấm force-push”, mất nhiều thời gian để kỹ sư học, và có xu hướng làm hỏng các comment code review hiện có trong UI GitHub. Dù vậy, với khoảng hai ba PR thì vẫn hoạt động tạm ổn
Trong luồng làm việc của chúng tôi, commit giống điểm lưu hơn là đơn vị công việc
Chúng tôi cũng dùng stash nhiều và dùng undo-tree-mode của Emacs nhiều. Như vậy có bốn cách để theo dõi lịch sử mã nguồn; nghe có vẻ trùng lặp, nhưng thực tế vận hành khá ổn
Tính khả dụng của Git khi xử lý chuyện này khá tệ. Phabricator có tốt hơn, nhưng vẫn khó một cách không cần thiết. Có lẽ một công cụ quản lý phiên bản mới có thể hỗ trợ các khái niệm cấp cao hơn commit và branch như công dân hạng nhất, hoặc cũng có thể như vậy lại khiến nó khó dùng hơn
Có muốn một công cụ tốt hơn Git không? Tất nhiên tôi cũng muốn một công cụ ít khuyết điểm hơn và thân thiện với người dùng hơn. Nhưng như một giải pháp tạm, sao không đối xử với commit như commit? Đừng tự làm khó mình không cần thiết
Tôi tự hỏi GitHub phải chịu bao nhiêu trách nhiệm trong việc khiến mọi người nhìn mọi thứ qua lăng kính PR
jj,sapling,mercurialBài blog hay, và thật vui khi thấy có những người thực sự suy nghĩ về cách code review nên vận hành
Tôi đã dùng nhiều bốn hệ thống code review có các ưu nhược điểm khác nhau: Critique (nội bộ Google), Gerrit (tôi dùng ở Google nhưng giống bản bên ngoài), GitHub, và CodeApprove do tôi tạo ra
Critique vượt trội nhất, nhưng điều đó khả thi vì nó được điều chỉnh hoàn hảo cho monorepo của Google, hệ thống quản lý phiên bản riêng, và các công cụ lint/test tùy chỉnh. CodeApprove được thiết kế để cố mang các ưu điểm đó sang GitHub nhiều nhất có thể, nhưng trên thực tế rất khó tiến gần đến mức đó
Gerrit đứng thứ hai về trải nghiệm của reviewer, nhưng với vai trò tác giả thì tôi luôn ghét nó. Có vẻ có nhiều cách làm sai hơn là cách làm đúng, và UI cũng không đẹp lắm
GitHub rất thân thiện với tác giả và hoạt động theo cách chúng ta suy nghĩ. Viết code, nhận phản hồi, rồi viết thêm code. Nếu squash and merge ở cuối PR thì không có vấn đề lịch sử như tác giả nói
Nhưng nó không thân thiện lắm với reviewer hay đội nhóm. Diff tăng dần không được nhấn mạnh, diff và cuộc thảo luận nằm ở các tab khác nhau, force push và rebase phá hủy lịch sử. Comment biến mất thành “outdated”, không thể comment vào file ngoài cửa sổ diff, file lớn bị ẩn theo mặc định, và còn nhiều vấn đề khác. Rõ ràng GitHub không mấy quan tâm chuyện này, và có lẽ họ biết điều gì đó mà tôi không biết
Cuối cùng, điều khó chịu nhất là nhiều đội cứ chấp nhận công cụ code review tích hợp sẵn trong nền tảng quản lý phiên bản. Nó giống như dùng nguyên IDE được cài sẵn trên laptop. Hiện nay có rất nhiều lựa chọn tốt hơn. Ngoài CodeApprove, những công cụ tôi thích là GitContext, Reviewable, Graphite, và tôi còn có thể kể thêm ít nhất nửa tá lựa chọn tuyệt vời khác. Đừng chấp nhận mặc định
Reviewer và maintainer khan hiếm hơn contributor rất nhiều. Tôi cho rằng workflow và UI nên tối ưu hóa thông lượng review
Tôi chưa dùng ba công cụ còn lại được nhắc đến, nên lập luận của tôi là nhận xét chung