Published on: 2013-10-05T11:36:26+00:00
In an email conversation, Gavin Andresen and another participant discussed the issue of incentivizing code review for open-source software projects. They considered using better tools to help with incentivization but expressed concerns over making the pool of reviewers even smaller if the tools are too difficult to use. They also debated whether to thank only people who significantly helped test or review other people's code in future releases' "Thank you" sections or to have a separate section for those who helped review above the current section. Ultimately, they decided it would be unfair not to credit occasional contributors who fixed bugs or maintained something important but didn't review complicated changes to the core.The email thread from October 2013 on the Bitcoin-development mailing list involved Gavin Andresen discussing the process of submitting large and complex work for review. Mike Hearn requested that such work either be submitted as one squashed change or kept logically clean and separated. Andresen agreed to try harder to keep commits separate and thanked Hearn for reviewing the fee changes in detail. During the discussion, Andresen mentioned the idea of using Review Board but expressed concern about potential reviewers being deterred by having to sign up for another account or learn a new tool. He asked if there were examples of other open source projects successfully incentivizing code review. Warren recommended reading the kernel.org documentation on submitting patches for helpful information.In another email exchange, Mike Hearn requested that when submitting large, complex pieces of work for review, they should either be submitted as one giant squashed change or be careful about keeping commits logically clean and separated. Gavin Andresen agreed to try harder to keep commits logically clean and separated. They also discussed the use of Review Board, a tool for code review. Andresen expressed his support for better tools but worried that requiring potential reviewers to sign up and learn another tool may make the pool of reviewers even smaller. They also looked for examples of how other open source software projects incentivize code review and suggested that only people who significantly helped test or review other people's code should be thanked in the "Thank You" section for future releases.In a discussion about code review, it was emphasized that making commits serves to explain to other coders why a change was made. The number of commits is not set in stone, but it is important to consider what would be the best way to explain the changes to fellow coders. It is also crucial to ensure that every commit works and doesn't break the build, as git-bisect works best if every commit in the tree being debugged works without errors. Squashing all changes into one big commit may result in a lack of explanation for why changes were made. While individual commits can be useful for understanding why someone made a change, it may be better to encourage better commit behavior rather than squashing chains of commits.The discussion revolved around whether it is better to have a long chain of commits on a feature branch that are compressed into one big diff or to preserve the individual and accurate messages in the individual commits. The first option disregards valuable information and hinders easy reviewing, while the second option maintains advantages like easier merges and bisects. However, reviewing lots of small commits can be challenging, and some prefer to review large diffs. It is suggested not to discard the small commits, as they can be useful in isolating specific changes. The writer personally puts effort into creating well-described commits and believes that collapsing them all would be a waste of effort.In October 2013, Mike Hearn proposed using Github as a third-party for Bitcoin development, but raised concerns about potentially creating the impression of a closed development process by moving the code review to a server controlled with explicit review groups. He also mentioned Fossil as an alternative option.In an email exchange between Peter Todd and another contributor, Todd shared his preference for using the "Files Changed" tab on GitHub when reviewing multiple commit pull-requests as it allows him to see every change made. The other contributor mentioned that comments on the files changed tab have disappeared in the past, leading to a loss of trust in doing reviews that way. However, they acknowledged that it does make things easier to read. They also discussed the advantages and risks of using GitHub as an independent third party for code review, including the potential impression of a closed process and the possibility of allowing anyone to sign up and comment.In a discussion about code review on Bitcoin, Peter Todd highlighted the issue of rebasing pull requests multiple times until they are accepted, which can render earlier code reviews irrelevant. He also mentioned the risk of introducing malicious code into the Bitcoin codebase. Arto Bendiken provided an example from 2003 where a missing character was used to backdoor the Linux kernel, emphasizing the need for careful review.
Updated on: 2023-08-01T05:58:44.560800+00:00