So sánh Unified Diff và Split Diff

 (matklad.github.io)
3 điểm bởi GN⁺ 2023-10-25 | 1 bình luận | Chia sẻ qua WhatsApp
  • Với các thay đổi nhỏ, cả unified diff lẫn split diff đều đủ để xem, nhưng với các thay đổi lớn và phức tạp thì việc review chỉ bằng diff sẽ chạm tới giới hạn
  • Với thay đổi lớn, cần khám phá và kiểm chứng dựa trên trạng thái tổng thể của codebase tại một thời điểm cụ thể, giống như đang chỉnh sửa mã thực tế, hơn là chỉ dựa vào lịch sử thay đổi
  • Màn hình review lý tưởng nên hiển thị song song toàn bộ mã hiện có trên đĩa và unified diff của khu vực tương ứng, để có thể theo dõi đồng thời ngữ cảnh mã và nội dung thay đổi
  • Vì các công cụ hiện có hỗ trợ chưa tốt, tác giả dùng một workflow công nghệ thấp: checkout PR về local, rồi xóa các commit và chỉ giữ lại phần thay đổi
  • Cách này giúp duyệt các file đã thay đổi và đánh dấu các hunk đã review, nhưng vẫn còn thao tác thủ công vì vị trí hiện tại trong editor và diff không tự động đồng bộ

Review theo trạng thái mã hiện tại thay vì chỉ nhìn diff

  • split diff và unified diff đều hoạt động tốt với các thay đổi nhỏ và đơn giản
  • Khi thay đổi trở nên lớn và phức tạp hơn, chỉ đọc diff thôi là chưa đủ
  • Với thay đổi lớn, cần xem xét kỹ codebase tại một thời điểm cụ thể
    • Vẫn tập trung vào các vùng vừa thay đổi, nhưng về cơ bản phải xem như một buổi code review thông thường
    • Cần chạy test
    • Cần dùng các tính năng điều hướng của editor như goto definition
    • Cần áp dụng thay đổi local để kiểm tra xem có cách viết khác tốt hơn không
    • Cần tìm những phần lẽ ra cũng nên được thay đổi trong ngữ cảnh rộng hơn
    • Cần có khả năng phát hiện những điểm kỳ lạ trong codebase hiện tại, bất kể lịch sử thay đổi ra sao

Màn hình review mong muốn và cách lách qua local

  • Diff view lý tưởng là bên trái hiển thị trạng thái mã hiện tại, bên phải hiển thị unified diff của đúng vùng mã đang nhìn thấy
    • Mã bên trái phải giống hệt trạng thái hiện có trên đĩa
    • Các thay đổi được đánh dấu nhẹ nhàng ở lề
    • Diff bên phải phải tương ứng với vùng mã hiện đang hiển thị ở bên trái
  • Hiện tại các công cụ chưa hỗ trợ tốt kiểu review này
  • Workflow thực tế là dùng gpr script để checkout PR về local
    • Ở bước cuối, xóa các commit của PR và chỉ giữ lại phần thay đổi
    • Trong trạng thái này, workflow dùng cho staging và commit được tận dụng cho code review
  • Với edamagit, có thể xem danh sách các file đã thay đổi và di chuyển tới thay đổi kế tiếp/trước đó trong editor
    • Cũng có thể dùng staging area để đánh dấu các hunk đã review
  • Điểm bất tiện còn lại là magit status buffer và file hiện đang mở trong editor không tự động đồng bộ
    • Muốn xem file hiện tại và diff cạnh nhau thì phải tự mở diff và cuộn thủ công tới đúng vị trí đang xem
  • Để làm kiểu review sát với mã đến mức này, hiện giờ vẫn phải tự chế các công cụ tạm thời
  • Mục tiêu chính của code review không nhất thiết chỉ là kiểm tra mã; có thể đọc thêm bài liên quan Two Kinds of Code Review

Bài viết liên quan

1 bình luận

 
GN⁺ 2023-10-25
Ý kiến trên Hacker News

  • Việc nói rằng với những thay đổi lớn, người ta không muốn review diff mà muốn review đúng codebase tại một thời điểm cụ thể và chú ý đến các vùng thay đổi gần đây, cá nhân tôi thấy không phù hợp với code review thông thường
    Tùy theo từng đội và từng ticket, nhưng nếu reviewer không phải là vai trò kiểu principal engineer chịu trách nhiệm chính cho toàn bộ mã nguồn, thì thông thường code review giống một bước kiểm tra tính nhất quán đơn giản bằng một cặp mắt thứ hai hơn
    Nếu các commit trong PR được sắp xếp gọn gàng hoặc squash lại, việc xem riêng các thay đổi tăng dần cũng tương đối dễ
    Rốt cuộc, sự bất mãn này có vẻ gần với khác biệt về loại review hơn. Có review kiểm tra tính nhất quán phổ biến, và review sâu về kiến trúc/chức năng trước khi merge; code review trên các công cụ web phần lớn có vẻ gần với loại đầu hơn

    • Xem code review chỉ là kiểm tra tính nhất quán là một góc nhìn quá nghèo nàn. Code review là cơ chế cốt lõi để giảm quyền sở hữu mã mang tính cá nhân, lan truyền quy ước và thực hiện chuyển giao kỹ thuật
      Một code review tốt bắt đầu từ một PR tốt. PR phải giải thích mục tiêu và cách đạt được mục tiêu đó
      Mục đầu tiên của một review tốt là xác nhận rằng mã đạt được mục tiêu đã mô tả; thứ hai là có test để kiểm chứng chức năng được tuyên bố hay không; thứ ba là có tuân theo quy ước kiến trúc của hệ thống hiện có hay không; thứ tư là style có khớp với kỳ vọng hay không; và cuối cùng là có đề xuất nào để cải thiện việc sử dụng API hay không
      Code review chỉ kiểm tra tính nhất quán thì ít hữu ích, vì CI cũng có thể làm việc đó. Code review là một quy trình của con người, nên cần tận dụng tối đa những việc chỉ con người mới làm được, phần còn lại hãy giao cho máy
      Trong các mục trên có ngầm chứa câu hỏi: “Thay đổi này có trở thành một ví dụ tốt cho các đóng góp trong tương lai không?”
    • Tôi cho rằng code review hời hợt chỉ tạo ra kết quả hời hợt. Đây là lý do vì sao dù code review rất phổ biến, vẫn có nhiều bug được deploy
      Tôi luôn code review theo góc nhìn của tester. Tôi build và chạy code để xem nó có hoạt động như dự định không, tìm điểm yếu và cố làm nó hỏng
      Diff không phải là điểm kết thúc mà là điểm bắt đầu. Nó chỉ là manh mối cho biết nên chọc vào đâu trong code; cuối cùng, bằng chứng đến từ hành vi thực tế
      Có thể nói “đó là việc của tester, không phải của engineer”, nhưng tôi tự hỏi ai có thể test tốt hơn người hiểu code và biết phải nhìn vào đâu
      Tôi biết thái độ này thuộc thiểu số. Nhiều engineer có vẻ phản cảm một cách kỳ lạ với việc tự chạy code, thậm chí cả dùng debugger. Họ xây những hạ tầng khổng lồ để tự động hóa mọi thứ, nhưng tự động hóa rốt cuộc cũng chỉ là code có thể có bug. Ai sẽ test các bài test, Quis custodiet ipsos custodes?
    • Tôi đồng ý với những gì nên kỳ vọng ở code review, nhưng để nó vận hành đúng thì trước hết cần có một cách làm việc nhất định
      Code review là tuyến phòng thủ cuối cùng nếu không tính CI, nhưng trong các đội tôi từng làm, hướng đi lớn mà PR đưa vào đã được thảo luận với nhiều người trước thời điểm đó. Có thể là pair programming, giải thích sâu, coffee chat, hay bất cứ cách nào phù hợp với độ phức tạp
      Khi đó PR trở thành nơi review chiến thuật triển khai chiến lược đó, chứ không phải bản thân chiến lược như tách module mới, đưa vào dependency lớn, hay làm lại bề mặt API
      Nếu chỉ kiểm tra tính nhất quán một phần code mà không có giao tiếp từ trước, có nguy cơ quyền sở hữu mã bị chia nhỏ. Nó sẽ thành tình trạng kiểu “Module đó làm gì vậy?” “Không biết, hỏi Bob đi”
      Riêng chuyện đó sang một bên, dù ở chế độ review nào thì tôi vẫn thích ý tưởng về diff view mà tác giả đề xuất
    • Có những ý hay, nhưng diff review khi gặp đi gặp lại thì nhiều trường hợp đơn giản là không thể hiểu nổi, nên trở nên khá vô nghĩa
      Với một khối các dòng đỏ và xanh lẫn lộn, rất khó thực hiện bất kỳ review nào. Đó không phải là một góc nhìn hữu ích về code, và ít nhất thì cũng không phù hợp làm view mặc định
      Dù vậy vì là công việc nên vẫn phải cố làm, nhưng tôi đồng ý 100% với tác giả rằng đó không phải là một view thật sự hữu ích
    • Văn hóa của các công ty công nghệ hiện đại đã đi quá xa khỏi những quy trình hình thức hơn để quản lý phát triển
      Vì không có quy trình cơ bản để xử lý defect, tính năng, v.v., mỗi engineer lại phải tự phát minh cách phối hợp để đưa tính năng ra production mỗi lần
      Phần lớn cuối cùng gần như chẳng làm gì, và code review trở thành hoạt động hợp tác duy nhất diễn ra ở mức chi tiết có ý nghĩa về chủ đề đó. Vì vậy, mọi can thiệp có thể đều bị nhồi vào cùng một thời điểm

  • Cũng đáng nhắc đến difftastic như lựa chọn thứ ba, hoặc thứ tư. Nó dùng diff “cấu trúc” thay vì diff theo dòng, nên làm nổi bật các thay đổi chi tiết hơn
    https://github.com/Wilfred/difftastic

    • Là lựa chọn thứ tư, hoặc thứ năm, còn có patdiff: https://opensource.janestreet.com/patdiff/
      Theo trí nhớ của tôi, đôi khi nó làm khoảng 35% diff dễ đọc hơn, thường thì 60% không khác gì, và hiếm khi khoảng 5% lại khó đọc hơn
      Tôi dùng nó vài năm trước nên không nhớ rõ các vấn đề cụ thể. Lý do duy nhất tôi ngừng dùng là vì bắt đầu dùng magit cho Git diff
    • Đây là một cách tiếp cận thực sự tốt. Chỉ tiếc là nó chưa được tích hợp rộng rãi. Theo tôi biết, đầu ra JSON có cấu trúc đúng nghĩa để các công cụ khác sử dụng cũng chỉ mới có gần đây
    • ediff của Emacs cũng làm việc này. Tính năng tô sáng các từ khác nhau là Refine
    • git diff --word-diff cũng có chức năng tương tự và khá tốt

  • Khi review bằng Vim cũng có cảm giác tương tự
    Có một script nhỏ để mở PR, và với các file đã thay đổi thì nó thực hiện thao tác đại khái như vimdiff <(git show baseref:file) file. Tab của Vim về thực chất là view, nên có thể mở đồng thời từng buffer ở các trạng thái khác nhau, khá tiện
    Ở view mặc định, khóa cuộn hoạt động như mong đợi, và nếu cần thì có thể tránh nó ở tab riêng
    [c và ]c di chuyển giữa các hunk dựa trên các cụm thay đổi có trong PR, chứ không phải thư mục làm việc
    Có thể dùng dp và dg để push/pull hunk vào buffer diff chỉ đọc và đánh dấu là “xong”; khi đó nó sẽ được ẩn khỏi các thay đổi được tô sáng trong buffer live
    Thay đổi trong buffer live có thể được commit ngay hoặc push dưới dạng comment
    Các tính năng editor thông thường như go to definition, tích hợp build, tài liệu popup cũng vẫn hoạt động trên trạng thái cây hiện tại
    Cách này vừa xem được thay đổi so với nhánh cơ sở của PR, vừa giữ thư mục làm việc sạch. Trông tốt hơn nhiều so với giải pháp của matklad là xem thay đổi trong trạng thái làm bẩn thư mục làm việc
    Trong môi trường tôi làm việc, thay đổi thường được thêm dưới dạng commit --fixup, và công cụ sẽ gộp lại ở thời điểm merge bằng cách gọi git-interpret-trailers để quy tác giả commit fixup về commit gốc, nên cách này đặc biệt hay. Nó cũng trích các comment văn bản của PR và nếu phù hợp thì thêm trailer Reviewed-by, hoặc nếu tương ứng với +1 thì thêm trailer Acked-by

    • Tôi tò mò không rõ “push dưới dạng comment” và “trích các comment văn bản” chính xác nghĩa là gì. Có phải kiểu logic tùy biến chỉ dùng nội bộ công ty không?

  • Với những codebase rất lớn và phức tạp có nhiều kỹ sư tham gia, đây là một vấn đề lớn
    Code review khó là vì diff lúc nào trông cũng hợp lý, test lúc nào cũng pass, và những thứ cơ bản thì luôn đã được kiểm tra
    Nhưng dù thay đổi trông có vẻ hợp lý, thực tế nó vẫn thường sai
    Chỉ một thay đổi tệ nhưng trông hợp lý cũng có thể khiến toàn bộ kiến trúc bị trôi dạt
    Như thường lệ, nói chính xác thì đây không hẳn là vấn đề công cụ mà gần với vấn đề văn hóa và chia sẻ tri thức hơn. Chỉ có công cụ tốt hơn thì không giải quyết được

    • Khó chấp nhận đồng thời hai câu “test lúc nào cũng pass” và “thay đổi trông hợp lý nhưng thường sai”
      Khi code review, hiếm khi ta tìm chính bug. Thay vào đó, ta xem liệu có test để bắt được những bug như vậy không

  • Có vẻ tôi đang bỏ lỡ điều gì đó. Tác giả nói split diff không hợp với mình nhưng không nói lý do
    Diff lý tưởng trông gần như giống split diff đã loại bỏ ngữ cảnh trùng lặp. Thay vì ngữ cảnh bị lặp ở cả hai bên, nó chỉ nằm ở bên trái
    Tôi tò mò việc loại bỏ ngữ cảnh trùng lặp ở panel bên phải mang lại lợi ích gì

    • Tác giả viết rằng họ cần chạy test, dùng go to definition và các tính năng điều hướng editor khác, áp dụng thay đổi cục bộ để kiểm tra liệu có thể viết theo cách khác không, xem ngữ cảnh rộng hơn để tìm những phần đáng lẽ cũng phải thay đổi, và nhận ra các điểm kỳ lạ trong codebase hiện tại bất kể lịch sử thay đổi diễn ra thế nào
      Các editor/IDE tôi từng dùng thường chỉ hỗ trợ những tính năng này ở mức cơ bản trong diff. Go to definition cũng chỉ hoạt động trong cùng file, autocomplete thường không tốt với các mục mới thêm, và nhiều khi không có tính năng refactoring
    • Bên trái hiển thị code hiện tại không chỉnh sửa. Nếu một bên có toàn bộ ngữ cảnh kèm tính năng điều hướng code, còn bên kia hiển thị thay đổi, bạn có thể thấy thay đổi nào được áp dụng trong một ngữ cảnh cụ thể
      Theo tôi hiểu, tác giả muốn đảo ngược diff. Thay vì nhìn vào chính thay đổi, họ muốn nhìn vào code rồi kiểm tra xem có thay đổi liên quan ở đó không
      Bài viết thiếu chi tiết, nhưng có vẻ họ muốn điều hướng tới phần triển khai hoặc các điểm gọi rồi kiểm tra xem có thay đổi phù hợp không
    • Tôi không phải tác giả nhưng thấy đồng cảm
      Tô sáng diff có thể gây xao nhãng hơn tưởng tượng, và đôi khi tắt nó đi thì độ dễ đọc cải thiện rõ rệt. Tôi cũng thỉnh thoảng tắt trong diff viewer để nhìn code bằng con mắt mới, nhưng không có tùy chọn hiển thị unified diff cùng lúc trong split pane

  • Trên GitHub, nhấn . sẽ đưa bạn vào một IDE đầy đủ trong trình duyệt
    Khi review, việc này rất hữu ích. Thay vì chỉ xem từng mẩu, bạn có thể xem thay đổi trong ngữ cảnh toàn bộ file, nên bắt được vấn đề thiết kế tinh vi tốt hơn nhiều

    • Cái này thật sự hay. Tôi chỉ từng vào IDE này khi sửa file trực tiếp trên GitHub, không biết là có phím tắt đi thẳng từ commit diff
      Nhân tiện, phím tắt này cũng hoạt động trên GitLab

  • Tôi nhớ một số tính năng của p4merge. Ví dụ có tính năng hiển thị ở khoảng trống giữa split diff rằng phần nào của file này tương ứng với phần nào của file kia, đồng thời cho thấy ảnh hưởng của diff ở cả hai bên: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
    Tôi cũng thích tính năng 3-way merge: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
    Những tính năng này chưa xuất hiện trong các công cụ diff nền web được dùng rộng rãi. Tôi nghĩ 3-way diff là cách tốt để hiển thị kết quả của một merge khó

  • Nếu dùng GitHub, trên trang pull request của github.com bạn có thể nhấn . hoặc đổi domain thành github.dev
    Khi đó VSCode sẽ mở trong trình duyệt, và pull request được hiển thị dưới dạng diff view
    Ưu điểm là có thể xem toàn bộ file, và vì thuật toán diff khác với github.com nên đôi khi dễ đọc hơn với diff phức tạp

    • Nhược điểm là chậm và nhiều bug
      Side-by-side diff view hiển thị comment PR thành danh sách ngoài dòng có thể rất hữu ích, nhưng thật đáng tiếc là phải mở cả một instance VSCode trong trình duyệt
      Thực ra nó hữu ích đến mức trước đây GitHub từng có split view như vậy, và không cần đẩy mọi người vào một mớ buzzword kiểu môi trường phát triển hỗ trợ AI/ML/crypto/blockchain/cloud
      Ý tưởng thì hay nhưng triển khai quá tệ. Có lẽ câu này cũng tóm tắt khá đúng GitHub nói chung hiện nay

  • Meld khá phù hợp cho mục đích này
    https://meldmerge.org/

    • https://kdiff3.sourceforge.net/ cũng có nhiều kiểu hiển thị tốt nên đáng được nhắc đến cùng
    • Trông khá giống FileMerge được bao gồm trong bộ công cụ dành cho nhà phát triển của Apple

  • Công cụ tôi thích nhất là diff2html-cli: https://diff2html.xyz/
    https://www.npmjs.com/package/diff2html-cli
    Có thể xem diff dưới dạng HTML, hỗ trợ dạng side-by-side hoặc unified