README+docs: add code review checklist

We want to encourage new contributors to review code instead of creating
their own PRs as a first contribution.

docs/review.md

@@ -0,0 +1,139 @@
+# Code review
+
+This document aims to explain why code review can be a very valuable form of
+contribution to any Open Source project.
+But code review is a very broad term that can mean many things. Good in-depth
+code review is also a skill that needs to be learned but can be highly valuable
+in any development team.
+
+## Why should new contributors review code first?
+
+Developers interested in contributing to an Open Source project seem to often
+think that _writing code_ is the bottleneck in those projects.
+That rarely is the case though. Especially in Bitcoin related projects, where
+any mistake can have enormous consequences, any code that is to be merged
+needs to go through a thorough review first (often requiring two or more
+developers to approve before it can be merged).
+
+Therefore, the actual bottleneck in Bitcoin related Open Source projects is
+**review, not the production of code**.
+
+So new contributors that come into a project by opening a PR often take more
+time away from the maintainers of that project than what their first
+contribution ends up adding in value. That is especially the case if the
+contributors don't have a lot of experience with git, GitHub, the target
+programming language and its formatting requirements or the review and test
+process as a whole.
+
+If a new contributor instead starts reviewing Pull Requests first, before
+opening their own ones, they can gain the following advantages:
+ - They learn about the project, its formal requirements and the programming
+   language used, without anyone's assistance.
+ - They learn about how to compile, execute and test the project they are aiming
+   to contribute to.
+ - They see best practices in terms of formatting, naming, commit structure,
+   etc.
+ - The value added to the project by reviewing, running and testing code is
+   likely positive from day one and requires much less active assistance and
+   time from maintainers.
+
+The maintainers of this project therefore ask all new contributors to review
+at least a couple of Pull Requests before adding their own.
+
+## How do I review code?
+
+Reviewing code in a constructive and helpful way is almost an art form by
+itself. The following list is definitely not complete and every developer tends
+to develop their own style when it comes to code review. But the list should
+have the most basic tasks associated with code review that should be a good
+entry point for anyone not yet familiar with the process:
+
+- Make sure to pick a Pull Request that fits your level of experience. You
+  might want to pick one that [has the "good first review"
+  label](https://github.com/lightningnetwork/lnd/labels/good%20first%20review).
+  If you're new to the area that you'd like to review, first familiarize
+  yourself with the code, try to understand the architecture, public interfaces,
+  interactions and code coverage. It can be useful to look at previous PRs to
+  get more insight.
+- Check out the code of the pull request in your local git (check [this
+  guide](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally)
+  for an easy shortcut).
+- Compile the code. See [INSTALL.md](INSTALL.md) and [MAKEFILE.md](MAKEFILE.md)
+  for inputs on how to do that.
+- Run the unit tests of the changed areas. See the [`unit` section of
+  MAKEFILE.md](MAKEFILE.md#unit) on how to run specific packages or test cases
+  directly.
+- Run the code on your local machine and attempt to test the changes of the
+  pull request. For any actual Bitcoin or Lightning related functionality, you
+  might want to spin up a `regtest` local network to test against. You can check
+  out the [`regtest` setup project](https://github.com/guggero/rt) that might
+  be helpful for that.
+- Try to manually test the changes in the PR. What to actually test might differ
+  from PR to PR. For a bug fix you might first want to attempt to reproduce the
+  bug in a version prior to the fix. Then test that the Pull Request actually
+  fixes the bug.
+  Code that introduces new features should be tested by trying to run all the
+  new code paths and try out all new command line flags as well as RPC
+  methods and fields. Making sure that invalid values and combinations lead to
+  proper error messages the user can easily understand is important.
+- Keep the logs of your tests as part of your review feedback. If you find a
+  bug, the exact steps to reproduce as well as the local logs will be super
+  helpful to the author of the Pull Request.
+- Propose areas where the unit test coverage could be improved (try to propose
+  specific test cases, for example "we should test the behavior with a negative
+  number here", instead of just saying "this area needs more tests").
+- Bonus: Write a unit test (or test case) that you can propose to add to the PR.
+- Go through the PR commit by commit and line by line
+    - Is the flow of the commits easy to understand? Does the order make sense?
+      See the [code contribution guidelines](code_contribution_guidelines.md)
+      for more information on this.
+    - Is the granularity of the commit structure easy to understand? Should
+      commits be split up or be combined?
+    - What does each line achieve in terms of the overall goal of the PR?
+    - What does each line change in the behavior of the code?
+    - Are there any potentially unwanted side effects?
+    - Are all errors handled appropriately?
+    - Is there sufficient logging to trace what happens if a user runs into
+      problems with the code? (e.g. when testing the code, can you find out what
+      code path your execution took just by looking at the trace-level log?)
+    - Does the commit message appropriately reflect the changes in that commit?
+      Only relevant for non-self-explanatory commits.
+- Is the documentation added by the PR sufficient?
+- Bonus: Propose additional documentation as part of the review that can be
+  added to the PR.
+- What does the user interface (command line flags, gRPC/REST API) affected by
+  the PR look like? Is it easy to understand for users? Are there any unintended
+  breaking changes for current users of the API being introduced? We attempt to
+  not break existing API users by renaming or changing existing fields.
+- Does the CI pass for the PR? If not, can you find out what needs to be changed
+  to make the CI pass? Unfortunately there are still some flakes in the tests,
+  so some steps might fail unrelated to the changes in the Pull Request.
+
+## How to submit review feedback
+
+The above checklist is very detailed and extensive. But a review can also be
+helpful if it only covers some of the steps outlined. The more, the better, of
+course, but any review is welcome (within reason of course, don't spam pull
+requests with trivial things like typos/naming or obvious AI-generated review
+that doesn't add any value).
+
+Just make sure to mention what parts you were focusing on.
+The following abbreviations/acronyms might be used to indicate partial or
+specific review:
+
+- Examples:
+  - cACK (concept acknowledgement): I only looked at the high-level changes, the
+    approach looks okay to me (but didn't deeply look at the code or tests).
+  - tACK (tested acknowledgement): I didn't look at the code, but I compiled,
+    ran and tested the code changes.
+  - ACK ("full" acknowledgement): I reviewed all relevant aspects of the PR and
+    I have no objections to merging the code.
+  - Request changes: I think we should change the following aspects of the PR
+    before merging: ...
+  - NACK (negative acknowledgement): I think we should _not_ proceed with the
+    changes in this PR, because of the following reasons: ... 
+
+Then try to give as much detail as you can. And as with every communication in
+Open Source projects: Always be polite and objective in the feedback.
+Ask yourself: Would you find the feedback valuable and appropriate if someone
+else posted it on your PR?