diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
index 2ca61829e..90f47ff81 100644
--- a/.github/pull_request_template.md
+++ b/.github/pull_request_template.md
@@ -1,5 +1,7 @@
#### Pull Request Checklist
+- [ ] If this is your first time contributing, we recommend you read the [Code
+ Contribution Guidelines](https://github.com/lightningnetwork/lnd/blob/master/docs/code_contribution_guidelines.md)
- [ ] All changes are Go version 1.11 compliant
- [ ] The code being submitted is commented according to the
[Code Documentation and Commenting](#CodeDocumentation) section
@@ -17,3 +19,7 @@
- [ ] Running `go vet` does not report any issues
- [ ] Running `make lint` does not report any **new** issues that did not
already exist
+- [ ] All commits build properly and pass tests. Only in exceptional
+ cases it can be justifiable to violate this condition. In that case, the
+ reason should be stated in the commit message.
+- [ ] Commits have a logical structure ([see section 4.5, of the Code Contribution Guidelines])
diff --git a/docs/code_contribution_guidelines.md b/docs/code_contribution_guidelines.md
index 1aaad73ba..1d06b5a75 100644
--- a/docs/code_contribution_guidelines.md
+++ b/docs/code_contribution_guidelines.md
@@ -7,8 +7,10 @@
4.2. [Testing](#Testing)
4.3. [Code Documentation and Commenting](#CodeDocumentation)
4.4. [Model Git Commit Messages](#ModelGitCommitMessages)
-4.5. [Code Spacing](#CodeSpacing)
-4.6. [Protobuf Compilation](#Protobuf)
+4.5. [Ideal Git Commit Structure](#IdealGitCommitStructure)
+4.6. [Code Spacing](#CodeSpacing)
+4.7. [Protobuf Compilation](#Protobuf)
+4.8. [Additional Style Constraints On Top of gofmt](ExtraGoFmtStyle)
5. [Code Approval Process](#CodeApproval)
5.1. [Code Review](#CodeReview)
5.2. [Rework Code (if needed)](#CodeRework)
@@ -164,6 +166,10 @@ A quick summary of test practices follows:
contained within `lnd`. For example integration tests, see
[`lnd_test.go`](https://github.com/lightningnetwork/lnd/blob/master/lnd_test.go#L181).
+Throughout the process of contributing to `lnd`, you'll likely also be
+extensively using the commands within our `Makefile`. As a result, we recommned
+[perusing the make file documentation](https://github.com/lightningnetwork/lnd/blob/master/docs/MAKEFILE.md).
+
#### 4.3. Code Documentation and Commenting
@@ -290,9 +296,44 @@ either a '+' or a ','. This prefix seems minor but can be extremely helpful in
determining the scope of a commit at a glance, or when bug hunting to find a
commit which introduced a bug or regression.
+
+
+#### 4.5. Ideal Git Commit Structure
+
+Within the project we prefer small, contained commits for a pull request over a
+single giant commit that touches several files/packages. Ideal commits build on
+their own, in order to facilitate easy usage of tools like `git bisect` to `git
+cherry-pick`. It's preferred that commits contain an isolated change in a
+single package. In this case, the commit header message should begin with the
+prefix of the modified package. For example, if a commit was made to modify the
+`lnwallet` package, it should start with `lnwallet: `.
+
+In the case of changes that only build in tandem with changes made in other
+packages, it is permitted for a single commit to be made which contains several
+prefixes such as: `lnwallet+htlcswitch`. This prefix structure along with the
+requirement for atomic contained commits (when possible) make things like
+scanning the set of commits and debugging easier. In the case of changes that
+touch several packages, and can only compile with the change across several
+packages, a `multi: ` prefix should be used.
+
+Examples of common patterns w.r.t commit structures within the project:
+
+ * It is common that during the work on a PR, existing bugs are found and
+ fixed. If they can be fixed in isolation, they should have their own
+ commit.
+ * File restructuring like moving a function to another file or changing order
+ of functions: with a separate commit because it is much easier to review
+ the real changes that go on top of the restructuring.
+ * Preparatory refactorings that are functionally equivalent: own commit.
+ * Project or package wide file renamings should be in their own commit.
+ * Ideally if a new package/struct/sub-system is added in a PR, there should
+ be a single commit which adds the new functionality, with follow up
+ induvidual commits that begin to intergrate the functionality within the
+ codebase.
+
-#### 4.5. Code Spacing
+#### 4.6. Code Spacing
Blocks of code within `lnd` should be segmented into logical stanzas of
operation. Such spacing makes the code easier to follow at a skim, and reduces
@@ -340,9 +381,67 @@ Functions should _not_ just be laid out as a bare contiguous block of code.
return witness
```
+Additionally, we favor spacing between stanzas within syntax like: switch case
+statements and select statements.
+
+**WRONG**
+```go
+ switch {
+ case a:
+
+ case b:
+
+ case c:
+
+ case d:
+
+ default:
+
+ }
+```
+**RIGHT**
+```go
+ switch {
+ // Brief comment detailing instances of this case (repeat below).
+ case a:
+
+
+ case b:
+
+
+ case c:
+
+
+ case d:
+
+
+ default:
+
+ }
+```
+
+If one is forced to wrap lines of function arguments that exceed the 80
+character limit, then a new line should be inserted before the first stanza in
+the comment body.
+**WRONG**
+```go
+ func foo(a, b, c,
+ d, e) error {
+ var a int
+ }
+```
+**RIGHT**
+```go
+ func foo(a, b, c,
+ d, e) error {
+
+ var a int
+ }
+```
+
-#### 4.5.6. Protobuf Compilation
+#### 4.7. Protobuf Compilation
The `lnd` project uses `protobuf`, and its extension [`gRPC`](www.grpc.io) in
several areas and as the primary RPC interface. In order to ensure uniformity
@@ -375,6 +474,38 @@ Notice how the `json_name` field option corresponds with the name of the field
itself, and uses a `snake_case` style of name formatting. All added or modified
`proto` fields should adhere to the format above.
+
+
+#### 4.8. Additional Style Constraints On Top of `gofmt`
+
+Before a PR is submitted, the proposer should ensure that the file passes the
+set of linting scripts run by `make lint`. These include `gofmt`. In addition
+to `gofmt` we've opted to enforce the following style guidelines.
+
+ * ALL columns (on a best effort basis) should be wrapped to 80 line columns.
+ Editors should be set to treat a tab as 4 spaces.
+ * When wrapping a line that contains a function call as the unwrapped line
+ exceeds the column limit, the close paren should be placed on its own
+ line. Additionally, all arguments should begin in a new line after the
+ open paren.
+
+ **WRONG**
+ ```go
+ value, err := bar(a,
+ a, b, c)
+ ```
+
+ **RIGHT**
+ ```go
+ value, err := bar(
+ a, a, b, c,
+ )
+ ```
+
+Note that the above guidelines don't apply to log messages. For log messages,
+committers should attempt to minimize the of number lines utilized, while still
+adhering to the 80-character column limit.
+
### 5. Code Approval Process
@@ -429,17 +560,27 @@ master. In certain cases the code reviewer(s) or interested committers may help
you rework the code, but generally you will simply be given feedback for you to
make the necessary changes.
+During the process of responding to review comments, we prefer that changes be
+made with [fixup commits](https://robots.thoughtbot.com/autosquashing-git-commits).
+The reason for this is two fold: it makes it easier for the reviewer to see
+what changes have been made between versions (since Github doesn't easily show
+prior versions like Critique) and it makes it easier on the PR author as they
+can set it to auto squash the fix up commits on rebase.
+
This process will continue until the code is finally accepted.
#### 5.3. Acceptance
-Once your code is accepted, it will be integrated with the master branch.
-Typically it will be rebased and fast-forward merged to master as we prefer to
-keep a clean commit history over a tangled weave of merge commits. However,
-regardless of the specific merge method used, the code will be integrated with
-the master branch and the pull request will be closed.
+Once your code is accepted, it will be integrated with the master branch. After
+2+ (sometimes 1) LGTM's (approvals) are given on a PR, it's eligible to land in
+master. At this final phase, it may be necessary to rebase the PR in order to
+resolve any conflicts and also squash fix up commits. Ideally, the set of
+[commits by new contributors are PGP signed](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work),
+although this isn't a strong requirement (but we prefer it!). In order to keep
+these signatures intact, we prefer using merge commits. PR proposers can use
+`git rebase --signoff` to sign and rebase at the same time as a final step.
Rejoice as you will now be listed as a [contributor](https://github.com/lightningnetwork/lnd/graphs/contributors)!
@@ -464,10 +605,14 @@ Rejoice as you will now be listed as a [contributor](https://github.com/lightnin
- [ ] For code and documentation: lines are wrapped at 80 characters
(the tab character should be counted as 8 characters, not 4, as some IDEs do
per default)
-- [ ] Running `go test` does not fail any tests
+- [ ] Running `make check` does not fail any tests
- [ ] Running `go vet` does not report any issues
-- [ ] Running [golint](https://github.com/golang/lint) does not
- report any **new** issues that did not already exist
+- [ ] Running `make lint` does not report any **new** issues that
+ did not already exist
+- [ ] All commits build properly and pass tests. Only in exceptional
+ cases it can be justifiable to violate this condition. In that case, the
+ reason should be stated in the commit message.
+- [ ] Commits have a logical structure (see section 4.5).
diff --git a/docs/ruby-thing.rb b/docs/ruby-thing.rb
new file mode 100644
index 000000000..922201fe3
--- /dev/null
+++ b/docs/ruby-thing.rb
@@ -0,0 +1,12 @@
+#!/usr/bin/env ruby
+
+File.open("INSTALL.md", 'r') do |f|
+ f.each_line do |line|
+ forbidden_words = ['Table of contents', 'define', 'pragma']
+ next if !line.start_with?("#") || forbidden_words.any? { |w| line =~ /#{w}/ }
+
+ title = line.gsub("#", "").strip
+ href = title.gsub(" ", "-").downcase
+ puts " " * (line.count("#")-1) + "* [#{title}](\##{href})"
+ end
+end