Proofread and fixed typos, etc. in contributing
This commit is contained in:
parent
56591db68d
commit
7d015b1528
125
CONTRIBUTING.md
125
CONTRIBUTING.md
|
@ -1,12 +1,7 @@
|
||||||
Contributing to Bitcore
|
Contributing to Bitcore
|
||||||
=======
|
=======
|
||||||
|
|
||||||
We're working hard to make *bitcore* the easier and most powerful javascript
|
We're working hard to make *bitcore* the easiest and most powerful javascript library for working with bitcoin. Our goal is to have *bitcore* be a library that can be used by anyone interested in bitcoin, and level the expertise differences with agreat design and documentation.
|
||||||
library for working with bitcoin. Our goal is to have *bitcore* be a library
|
|
||||||
that can be used by anyone interested on bitcoin, and level the expertise
|
|
||||||
differences with a great design and documentation.
|
|
||||||
|
|
||||||
We have a pretty strict set of guidelines for contributing.
|
|
||||||
|
|
||||||
## Quick checklist
|
## Quick checklist
|
||||||
|
|
||||||
|
@ -18,83 +13,59 @@ Make sure:
|
||||||
|
|
||||||
## Design Guidelines
|
## Design Guidelines
|
||||||
|
|
||||||
These are some global design goals in bitcore that any change must adhere to.
|
These are some global design goals in bitcore that any change must adhere.
|
||||||
|
|
||||||
### D1 - Naming Matters
|
### D1 - Naming Matters
|
||||||
|
|
||||||
We take our time with picking names. Code is going to be written once, and read
|
We take our time with picking names. Code is going to be written once, and read hundreds of times.
|
||||||
hundreds of times.
|
|
||||||
|
|
||||||
We were inspired to name this rule first due to Uncle Bob's great work *Clean
|
We were inspired to name this rule first due to Uncle Bob's great work *Clean Code*, which has a whole chapter on this subject.
|
||||||
Code*, which has a whole chapter on this subject.
|
|
||||||
|
|
||||||
In particular, you may notice that some names in this library are quite long
|
In particular, you may notice that some names in this library are quite long for the average JavaScript user. That's because we prefer a long but comprehensible name than an abbreviation that might confuse new users.
|
||||||
for the average javascript user. That's because we better have a long but
|
|
||||||
comprehensible name rather than an abbreviation that might confuse new users.
|
|
||||||
|
|
||||||
### D2 - Tests
|
### D2 - Tests
|
||||||
|
|
||||||
Write a test for all your code. We encourage Test Driven Development so we know
|
Write a test for all your code. We encourage Test Driven Development so we know when our code is right. We have increased test coverage from 80% to around 95% and are targeting 100% as we move towards our 1.0 release.
|
||||||
when our code is right. We migrated from our original code base with a 80% test
|
|
||||||
coverage and are targeting 100% as we move towards our 1.0 release.
|
|
||||||
|
|
||||||
### D3 - Robustness Principle
|
### D3 - Robustness Principle
|
||||||
|
|
||||||
*Be conservative in what you send, be liberal in what you accept.*
|
*Be conservative in what you send, be liberal in what you accept.*
|
||||||
|
|
||||||
Interfaces accept as much types of arguments as possible, so there's no mental
|
Interfaces should accept as many types of arguments as possible, so there's no mental tax on using them: we want to avoid questions such as "should I use a string here or a buffer?", "what happens if I'm not sure if the type of this variable is an Address instance or a string with it encoded in base-58?" or "what kind of object will I receive after calling this function?".
|
||||||
tax on using them: we want to avoid questions such as "should I use a string
|
|
||||||
here or a buffer?", "what happens if I'm not sure if the type of this variable
|
|
||||||
is an Address instance or a string with it encoded in base-58?" or "what kind
|
|
||||||
of object will I receive after calling this function?".
|
|
||||||
|
|
||||||
Accept a wide variety of usages and arguments, always return an internal kind
|
Accept a wide variety of use cases and arguments, always return an internal form of an object. For example, the class `PublicKey` can accept strings or buffers with a DER encoded public key (either compressed or uncompressed), another PublicKey, a PrivateKey, or a Point, an instance of the `elliptic.js` library with the point in bitcoin's elliptic curve that represents the public key.
|
||||||
of object. For example, the class `PublicKey` can accept strings or buffers
|
|
||||||
with a DER encoded public key (either compressed or uncompressed), another
|
|
||||||
PublicKey, a PrivateKey, or a Point, an instance of the `elliptic.js` library
|
|
||||||
with the point in bitcoin's elliptic curve that represents the public key.
|
|
||||||
|
|
||||||
### D4 - Consistency Everywhere
|
### D4 - Consistency Everywhere
|
||||||
|
|
||||||
Consistency on the way classes are used is paramount to allow an easier
|
Consistency on the way classes are used is paramount to allow an easier understanding of the library.
|
||||||
understanding of the library.
|
|
||||||
|
|
||||||
## Style Guidelines
|
## Style Guidelines
|
||||||
|
|
||||||
The design guidelines have quite a high abstraction level. These style
|
The design guidelines have quite a high abstraction level. These style guidelines are more concreate and easier to apply, and also more opinionated. The design guidelines mentioned above are the way we think about general software development and we believe they should be present in any software project.
|
||||||
guidelines are easier to detect and apply, and also more opinionated (the
|
|
||||||
design guidelines mentioned above are the way we think about general software
|
|
||||||
development and we believe they should be present on any software project).
|
|
||||||
|
|
||||||
### G0 - General: Default to felixge's style guide
|
### G0 - General: Default to Felixge's Style Guide
|
||||||
|
|
||||||
Follow this Node.js style guide: https://github.com/felixge/node-style-guide#nodejs-style-guide
|
Follow this Node.js Style Guide: https://github.com/felixge/node-style-guide#nodejs-style-guide
|
||||||
|
|
||||||
### G1 - General: No Magic Numbers
|
### G1 - General: No Magic Numbers
|
||||||
|
|
||||||
Avoid constants in the code as much as possible. Magic strings are also magic
|
Avoid constants in the code as much as possible. Magic strings are also magic numbers.
|
||||||
numbers.
|
|
||||||
|
|
||||||
### G2 - General: Internal Objects should be Instances
|
### G2 - General: Internal Objects should be Instances
|
||||||
|
|
||||||
If a class has a `publicKey` member, for instance, that should be a `PublicKey`
|
If a class has a `publicKey` member, for instance, that should be a `PublicKey` instance.
|
||||||
instance.
|
|
||||||
|
|
||||||
### G3 - General: Internal amounts must be integers representing Satoshis
|
### G3 - General: Internal amounts must be integers representing Satoshis
|
||||||
|
|
||||||
Avoid representation errors by always dealing with satoshis. For conversion for
|
Avoid representation errors by always dealing with satoshis. For conversion for frontends, use the `Unit` class.
|
||||||
frontends, use the `Unit` class.
|
|
||||||
|
|
||||||
### G4 - General: Internal network references must be Network instances
|
### G4 - General: Internal network references must be Network instances
|
||||||
|
|
||||||
A special case for [G2](#g2---general-internal-objects-should-be-instances) all
|
A special case for [G2](#g2---general-internal-objects-should-be-instances) all network references must be `Network` instances (see `lib/network.js`), but when returned to the user, its `.name` property should be used.
|
||||||
network references must be `Network` instances (see `lib/network.js`), but when
|
|
||||||
returned to the user, its `.name` property should be used.
|
|
||||||
|
|
||||||
### G5 - General: Objects should display nicely in the console
|
### G5 - General: Objects should display nicely in the console
|
||||||
|
|
||||||
Write a `.inspect()` method so an instance can be easily debugged in the
|
Write a `.inspect()` method so an instance can be easily debugged in the console.
|
||||||
console.
|
|
||||||
|
|
||||||
### G6 - General: Naming Utility Namespaces
|
### G6 - General: Naming Utility Namespaces
|
||||||
|
|
||||||
|
@ -127,27 +98,18 @@ These should have a matching static method that can be used for instantiation:
|
||||||
We've designed a structure for Errors to follow and are slowly migrating to it.
|
We've designed a structure for Errors to follow and are slowly migrating to it.
|
||||||
|
|
||||||
Usage:
|
Usage:
|
||||||
* Errors are generated in the file `lib/errors/index.js` by invoking `gulp
|
* Errors are generated in the file `lib/errors/index.js` by invoking `gulp errors`.
|
||||||
errors`.
|
|
||||||
* The specification for errors is written in the `lib/errors/spec.js` file.
|
* The specification for errors is written in the `lib/errors/spec.js` file.
|
||||||
* Whenever a new class is created, add a generic error for that class in
|
* Whenever a new class is created, add a generic error for that class in `lib/errors/spec.js`.
|
||||||
`lib/errors/spec.js`.
|
* Specific errors for that class should subclass that error. Take a look at the structure in `lib/errors/spec.js`, it should be clear how subclasses are generated from that file.
|
||||||
* Specific errors for that class should subclass that error. Take a look at the
|
|
||||||
structure in `lib/errors/spec.js`, it should be clear how subclasses are
|
|
||||||
generated from that file.
|
|
||||||
|
|
||||||
### E2 - Errors: Provide a getValidationError static method for classes
|
|
||||||
|
|
||||||
|
### E2 - Errors: Provide a `getValidationError` static method for classes
|
||||||
|
|
||||||
### I1 - Interface: Make Code that Fails Early
|
### I1 - Interface: Make Code that Fails Early
|
||||||
|
|
||||||
In order to deal with javascript's weak typing and confusing errors, we ask our
|
In order to deal with JavaScript's weak typing and confusing errors, we ask our code to fail as soon as possible when an unexpected input was provided.
|
||||||
code to fail as soon as possible when an unexpected input was provided.
|
|
||||||
|
|
||||||
There's a module called `util/preconditions`, loosely based on
|
There's a module called `util/preconditions`, loosely based on `preconditions.js`, based on `guava`, that we use for state and argument checking. It should be trivial to use. We recommend using it on all methods, in order to improve robustness and consistency.
|
||||||
`preconditions.js`, based on `guava`, that we use for state and argument
|
|
||||||
checking. It should be trivial to use. We recommend using it on all methods, in
|
|
||||||
order to improve robustness and consistency.
|
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
$.checkState(something === anotherthing, 'Expected something to be anotherthing');
|
$.checkState(something === anotherthing, 'Expected something to be anotherthing');
|
||||||
|
@ -158,15 +120,11 @@ $.checkArgumentType(something, PrivateKey); // but it's optional (will show up a
|
||||||
|
|
||||||
### I2 - Interface: Permissive Constructors
|
### I2 - Interface: Permissive Constructors
|
||||||
|
|
||||||
Most classes have static methods named `fromBuffer`, `fromString`, `fromJSON`.
|
Most classes have static methods named `fromBuffer`, `fromString`, `fromJSON`. Whenever one of those methods is provided, the constructor for that class should also be able to detect the type of the arguments and call the appropriate method.
|
||||||
Whenever one of those methods is provided, the constructor for that class
|
|
||||||
should also be able to detect the type of the arguments and call the
|
|
||||||
appropriate method.
|
|
||||||
|
|
||||||
### I3 - Interface: Method Chaining
|
### I3 - Interface: Method Chaining
|
||||||
|
|
||||||
For classes that have a mutable state, most of the methods that can be chained
|
For classes that have a mutable state, most of the methods that can be chained *SHOULD* be chained, allowing for interfaces that read well, like:
|
||||||
*SHOULD* be chained, allowing for interfaces that read well, like:
|
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
var transaction = new Transaction()
|
var transaction = new Transaction()
|
||||||
|
@ -200,8 +158,7 @@ function ImmutableClass(arg) {
|
||||||
|
|
||||||
### I5 - Interface: No new keyword for Constructors
|
### I5 - Interface: No new keyword for Constructors
|
||||||
|
|
||||||
Constructors should not require to be called with `new`. This rule is not
|
Constructors should not require to be called with `new`. This rule is not heavily enforced, but is a "nice to have".
|
||||||
heavily enforced, but is a "nice to have".
|
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
function NoNewRequired(args) {
|
function NoNewRequired(args) {
|
||||||
|
@ -214,23 +171,17 @@ function NoNewRequired(args) {
|
||||||
|
|
||||||
### T1 - Testing: Tests Must be Written Elegantly
|
### T1 - Testing: Tests Must be Written Elegantly
|
||||||
|
|
||||||
Style guidelines are not relaxed for tests. Tests are a good way to show how to
|
Style guidelines are not relaxed for tests. Tests are a good way to show how to use the library, and maintaining them is extremely necessary.
|
||||||
use the library, and maintaining them is extremely necessary.
|
|
||||||
|
|
||||||
Don't write long tests, write helper functions to make them be as short and
|
Don't write long tests, write helper functions to make them be as short and concise as possible (they should take just a few lines each), and use good variable names.
|
||||||
concise as possible (they should take just a few lines each), and use good
|
|
||||||
variable names.
|
|
||||||
|
|
||||||
### T2 - Testing: Tests Must not be Random
|
### T2 - Testing: Tests Must not be Random
|
||||||
|
|
||||||
Inputs for tests should not be generated randomly. Also, the type and structure
|
Inputs for tests should not be generated randomly. Also, the type and structure of outputs should be checked.
|
||||||
of outputs should be checked.
|
|
||||||
|
|
||||||
### T3 - Testing: Require 'bitcore' and look up classes from there
|
### T3 - Testing: Require 'bitcore' and look up classes from there
|
||||||
|
|
||||||
This helps to make tests more useful as examples, and more independent of where
|
This helps to make tests more useful as examples, and more independent of where they are placed. This also helps prevent forgetting to include all submodules in the bitcore object.
|
||||||
they are placed. This also helps prevent forgetting to include all submodules
|
|
||||||
in the bitcore object.
|
|
||||||
|
|
||||||
DO:
|
DO:
|
||||||
```javascript
|
```javascript
|
||||||
|
@ -244,9 +195,7 @@ var PublicKey = require('../lib/publickey');
|
||||||
|
|
||||||
## Pull Request Workflow
|
## Pull Request Workflow
|
||||||
|
|
||||||
Our workflow is based on GitHub's pull requests. We use feature branches,
|
Our workflow is based on GitHub's pull requests. We use feature branches, prepended with: `test`, `feature`, `fix`, `refactor`, or `remove` according to the change the branch introduces. Some examples for such branches are:
|
||||||
prepended with: `test`, `feature`, `fix`, `refactor`, or `remove` according to
|
|
||||||
the change the branch introduces. Some examples for such branches are:
|
|
||||||
```sh
|
```sh
|
||||||
git checkout -b test/some-module
|
git checkout -b test/some-module
|
||||||
git checkout -b feature/some-new-stuff
|
git checkout -b feature/some-new-stuff
|
||||||
|
@ -260,8 +209,7 @@ git remote add bitpay git@github.com:bitpay/bitcore.git
|
||||||
git pull --rebase bitpay master
|
git pull --rebase bitpay master
|
||||||
```
|
```
|
||||||
|
|
||||||
Note that we require rebasing your branch instead of mergeing it, for commit
|
Note that we require rebasing your branch instead of merging it, for commit readability reasons.
|
||||||
readability reasons.
|
|
||||||
|
|
||||||
After that, you can push the changes to your fork, by doing:
|
After that, you can push the changes to your fork, by doing:
|
||||||
```sh
|
```sh
|
||||||
|
@ -269,14 +217,9 @@ git push origin your_branch_name
|
||||||
git push origin feature/some-new-stuff
|
git push origin feature/some-new-stuff
|
||||||
git push origin fix/some-bug
|
git push origin fix/some-bug
|
||||||
```
|
```
|
||||||
Finally go to [github.com/bitpay/bitcore](https://github.com/bitpay/bitcore) in
|
Finally go to [github.com/bitpay/bitcore](https://github.com/bitpay/bitcore) in your web browser and issue a new pull request.
|
||||||
your web browser and issue a new pull request.
|
|
||||||
|
|
||||||
Main contributors will review your code and possibly ask for changes before
|
Main contributors will review your code and possibly ask for changes before your code is pulled in to the main repository. We'll check that all tests pass, review the coding style, and check for general code correctness. If everything is OK, we'll merge your pull request and your code will be part of bitcore.
|
||||||
your code is pulled in to the main repository. We'll check that all tests
|
|
||||||
pass, review the coding style, and check for general code correctness. If
|
|
||||||
everything is OK, we'll merge your pull request and your code will be part of
|
|
||||||
bitcore.
|
|
||||||
|
|
||||||
If you have any questions feel free to post them to
|
If you have any questions feel free to post them to
|
||||||
[github.com/bitpay/bitcore/issues](https://github.com/bitpay/bitcore/issues).
|
[github.com/bitpay/bitcore/issues](https://github.com/bitpay/bitcore/issues).
|
||||||
|
|
Loading…
Reference in New Issue