akovalenko/lnd.xprv
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #2323 from Roasbeef/update-contribution-guidelines

Browse Source 
Update contribution guidelines
This commit is contained in:
Olaoluwa Osuntokun 2018-12-19 17:28:24 -08:00 committed by GitHub
commit 9c0e0f445a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 175 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
.github/pull_request_template.md vendored
View File

@ -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])

169
docs/code_contribution_guidelines.md
View File

@ -7,8 +7,10 @@
4.2. [Testing](#Testing)<br />
4.3. [Code Documentation and Commenting](#CodeDocumentation)<br />
4.4. [Model Git Commit Messages](#ModelGitCommitMessages)<br />
4.5. [Code Spacing](#CodeSpacing)<br />
4.6. [Protobuf Compilation](#Protobuf)<br />
4.5. [Ideal Git Commit Structure](#IdealGitCommitStructure)<br />
4.6. [Code Spacing](#CodeSpacing)<br />
4.7. [Protobuf Compilation](#Protobuf)<br />
4.8. [Additional Style Constraints On Top of gofmt](ExtraGoFmtStyle)<br />
5. [Code Approval Process](#CodeApproval)<br />
5.1. [Code Review](#CodeReview)<br />
5.2. [Rework Code (if needed)](#CodeRework)<br />
@ -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).
<a name="CodeDocumentation" />
#### 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.
<a name="IdealGitCommitStructure" />
#### 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.
<a name="CodeSpacing" />
#### 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:
<code block>
case b:
<code block>
case c:
<code block>
case d:
<code block>
default:
<code block>
}
```
**RIGHT**
```go
switch {
// Brief comment detailing instances of this case (repeat below).
case a:
<code block>
case b:
<code block>
case c:
<code block>
case d:
<code block>
default:
<code block>
}
```
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
}
```
<a name="Protobuf" />
#### 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.
<a name="ExtraGoFmtStyle" />
#### 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.
<a name="CodeApproval" />
### 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.
<a name="CodeAcceptance" />
#### 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
- [&nbsp;&nbsp;] 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)
- [&nbsp;&nbsp;] Running `go test` does not fail any tests
- [&nbsp;&nbsp;] Running `make check` does not fail any tests
- [&nbsp;&nbsp;] Running `go vet` does not report any issues
- [&nbsp;&nbsp;] Running [golint](https://github.com/golang/lint) does not
report any **new** issues that did not already exist
- [&nbsp;&nbsp;] Running `make lint` does not report any **new** issues that
did not already exist
- [&nbsp;&nbsp;] 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.
- [&nbsp;&nbsp;] Commits have a logical structure (see section 4.5).
<a name="Licensing" />

12
docs/ruby-thing.rb Normal file
View File

@ -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