We love your input! We want to make contributing to this project as easy and transparent as possible, whether it's:
- Reporting a bug
- Discussing the current state of the code
- Submitting a fix
- Proposing new features
We use github to host code, to track issues and feature requests, as well as accept pull requests. Please report bugs using Github's issues.
We Use Github Flow, So All Code Changes Happen Through Pull Requests
Pull requests are the best way to propose changes to the codebase (we use Github Flow). We actively welcome your pull requests. Before you start writing the code, please make yourself familiar with JPL Coding standard and The power of ten. Even though not all rules from the standard will be enforced, it will give you a good idea of what is expected from the code before it can be merged.
- Fork the repo and create your branch from
master. - If you've added code that should be tested, add tests.
- If you've changed APIs, update the documentation.
- Ensure the test suite passes.
- Make sure your code lints.
- Issue that pull request!
All pull requests require the code to be reviewed and approved by another person in the team.
This is an example. Here's another example.
Great Bug Reports tend to have:
- A quick summary and/or background
- Steps to reproduce
- Be specific!
- Give sample code if you can.
- What you expected would happen
- What actually happens
- Notes (possibly including why you think this might be happening, or stuff you tried that didn't work)
(We included an autogenerated Bug Report template, when opening a new issue. Try it!)
We firmly believe in the share early, share often approach. The basic premise of the approach is to announce your plans before you start work, and once you have started working, craft your changes into a stream of small and easily reviewable commits.
This approach has several benefits:
- Announcing your plans to work on a feature before you begin work avoids duplicate work.
- It permits discussions which can help you achieve your goals in a way that is consistent with the existing architecture.
- It minimizes the chances of you spending time and energy on a change that might not fit with the consensus of the community or existing architecture and potentially be rejected as a result.
- Incremental development helps ensure you are on the right track with regards to the rest of the community.
- The quicker your changes are merged to master, the less time you will need to spend rebasing and otherwise trying to keep up with the main code base.
- At a minimum every function must be commented with its intended purpose and
any assumptions that it makes
- The general rule of thumb is to look at it as if you were completely unfamiliar with the code and ask yourself, would this give me enough information to understand what this function does and how I'd probably want to use it?
- Exported functions should also include detailed information the caller of the function will likely need to know and/or understand:
WRONG
// convert a compact uint32 to big.Int
func CompactToBig(compact uint32) *big.Int {RIGHT
// CompactToBig converts a compact representation of a whole number N to a
// big integer. The representation is similar to IEEE754 floating point
// numbers.
//
// Like IEEE754 floating point, there are three basic components: the sign,
// the exponent, and the mantissa. They are broken out as follows:
//
// * the most significant 8 bits represent the unsigned base 256 exponent
// * bit 23 (the 24th bit) represents the sign bit
// * the least significant 23 bits represent the mantissa
//
// -------------------------------------------------
// | Exponent | Sign | Mantissa |
// -------------------------------------------------
// | 8 bits [31-24] | 1 bit [23] | 23 bits [22-00] |
// -------------------------------------------------
//
// The formula to calculate N is:
// N = (-1^sign) * mantissa * 256^(exponent-3)
//
// This compact form is only used in bitcoin to encode unsigned 256-bit numbers
// which represent difficulty targets, thus there really is not a need for a
// sign bit, but it is implemented here to stay consistent with bitcoind.
func CompactToBig(compact uint32) *big.Int {- Comments in the body of the code are highly encouraged, but they should explain the intention of the code as opposed to just calling out the obvious
WRONG
// return err if amt is less than 5460
if amt < 5460 {
return err
}RIGHT
// Treat transactions with amounts less than the amount which is considered dust
// as non-standard.
if amt < 5460 {
return err
}NOTE: The above should really use a constant as opposed to a magic number, but it was left as a magic number to show how much of a difference a good comment can make.
https://www.conventionalcommits.org/en/v1.0.0-beta.3/#examples
The CI compiler has all warnings enabled and will fail if there is one. Do not silence the warnings with #pragma or similar. If you feel there's a reason to supress the warning, please discuss the issue with project maintainer. (Also see: JPL-LOC1.2)
As we are targeting embedded platforms, there shall be no dynamic memory allocation after the task is initialized. (Also see: JPL-LOC2.5)
Use static and dynamic asserts as a sanity checks. It's always better to fail early. (Also see: JPL-LOC3.16)
Please do not change the build/CI configuration without discussing the issue with the team. This also applies to any requirements or additional libraries.
In short, when you submit code changes, your submissions are understood to be under the Apache-2.0 License.
This document was adapted from the open-source contribution guidelines for Facebook's Draft JPL Coding standard and from Decred.