Code review



Summary:

The communication system Git, also used as a revision control system, allows forking of work and creating long chains of commits. However, when submitting large and complex pieces of work for review, it is requested to either submit it as one giant squashed change or use every single log message and diff, which are individually valuable. Every commit is two things: what source code has changed and a message explaining why the change was made, making them valuable in explaining the changes to other people working on the project. There is no hard and fast rule for committing, but putting oneself in fellow coders' shoes and explaining the best way to accomplish the task is necessary. It is important not to make commits that don't compile, and everything should be refactored to the point where the commit as a whole "works." In the case of Bitcoin, there are specific caveats; people tend to rebase their pull-requests over and over again until they are accepted, but that means earlier code review doesn't apply to later code pushed. Also, Bitcoin is a high-profile and high-profit target for those trying to get malicious code into the codebase. Therefore, reviewers may add additional commits for small changes instead of rebasing to clarify what changes were made so that code reviewers do not have to review the entire patch from scratch. The most eyes will look at the commits during the pull-req process, and after the code has been pulled, the audience for those commits is almost non-existent. It is suggested to review patches by looking at both individual commits to understand why someone wanted to make a change and all merged commits into one diff for a "what actually changed here?" review.


Updated on: 2023-06-07T17:26:04.819829+00:00