github.com/palcoin-project/palcd@v1.0.0/docs/code_contribution_guidelines.md (about)

     1  # Code contribution guidelines
     2  
     3  Developing cryptocurrencies is an exciting endeavor that touches a wide variety
     4  of areas such as wire protocols, peer-to-peer networking, databases,
     5  cryptography, language interpretation (transaction scripts), RPC, and
     6  websockets.  They also represent a radical shift to the current fiscal system
     7  and as a result provide an opportunity to help reshape the entire financial
     8  system.  There are few projects that offer this level of diversity and impact
     9  all in one code base.
    10  
    11  However, as exciting as it is, one must keep in mind that cryptocurrencies
    12  represent real money and introducing bugs and security vulnerabilities can have
    13  far more dire consequences than in typical projects where having a small bug is
    14  minimal by comparison.  In the world of cryptocurrencies, even the smallest bug
    15  in the wrong area can cost people a significant amount of money.  For this
    16  reason, the btcd suite has a formalized and rigorous development process which
    17  is outlined on this page.
    18  
    19  We highly encourage code contributions, however it is imperative that you adhere
    20  to the guidelines established on this page.
    21  
    22  ## Minimum Recommended Skillset
    23  
    24  The following list is a set of core competencies that we recommend you possess
    25  before you really start attempting to contribute code to the project.  These are
    26  not hard requirements as we will gladly accept code contributions as long as
    27  they follow the guidelines set forth on this page.  That said, if you don't have
    28  the following basic qualifications you will likely find it quite difficult to
    29  contribute.
    30  
    31  - A reasonable understanding of bitcoin at a high level (see the
    32    [Required Reading](#ReqReading) section for the original white paper)
    33  - Experience in some type of C-like language
    34  - An understanding of data structures and their performance implications
    35  - Familiarity with unit testing
    36  - Debugging experience
    37  - Ability to understand not only the area you are making a change in, but also
    38    the code your change relies on, and the code which relies on your changed code
    39  
    40  Building on top of those core competencies, the recommended skill set largely
    41  depends on the specific areas you are looking to contribute to.  For example,
    42  if you wish to contribute to the cryptography code, you should have a good
    43  understanding of the various aspects involved with cryptography such as the
    44  security and performance implications.
    45  
    46  ## Required Reading
    47  
    48  - [Effective Go](http://golang.org/doc/effective_go.html) - The entire btcd
    49    suite follows the guidelines in this document.  For your code to be accepted,
    50    it must follow the guidelines therein.
    51  - [Original Satoshi Whitepaper](http://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&cad=rja&ved=0CCkQFjAA&url=http%3A%2F%2Fbitcoin.org%2Fbitcoin.pdf&ei=os3VUuH8G4SlsASV74GoAg&usg=AFQjCNEipPLigou_1MfB7DQjXCNdlylrBg&sig2=FaHDuT5z36GMWDEnybDJLg&bvm=bv.59378465,d.b2I) - This is the white paper that started it all.  Having a solid
    52    foundation to build on will make the code much more comprehensible.
    53  
    54  ## Development Practices
    55  
    56  Developers are expected to work in their own trees and submit pull requests when
    57  they feel their feature or bug fix is ready for integration into the  master
    58  branch.
    59  
    60  ## Share Early, Share Often
    61  
    62  We firmly believe in the share early, share often approach.  The basic premise
    63  of the approach is to announce your plans **before** you start work, and once
    64  you have started working, craft your changes into a stream of small and easily
    65  reviewable commits.
    66  
    67  This approach has several benefits:
    68  
    69  - Announcing your plans to work on a feature **before** you begin work avoids
    70    duplicate work
    71  - It permits discussions which can help you achieve your goals in a way that is
    72    consistent with the existing architecture
    73  - It minimizes the chances of you spending time and energy on a change that
    74    might not fit with the consensus of the community or existing architecture and
    75    potentially be rejected as a result
    76  - Incremental development helps ensure you are on the right track with regards
    77    to the rest of the community
    78  - The quicker your changes are merged to master, the less time you will need to
    79    spend rebasing and otherwise trying to keep up with the main code base
    80  
    81  ## Testing
    82  
    83  One of the major design goals of all core btcd packages is to aim for complete
    84  test coverage.  This is financial software so bugs and regressions can cost
    85  people real money.  For this reason every effort must be taken to ensure the
    86  code is as accurate and bug-free as possible.  Thorough testing is a good way to
    87  help achieve that goal.
    88  
    89  Unless a new feature you submit is completely trivial, it will probably be
    90  rejected unless it is also accompanied by adequate test coverage for both
    91  positive and negative conditions.  That is to say, the tests must ensure your
    92  code works correctly when it is fed correct data as well as incorrect data
    93  (error paths).
    94  
    95  Go provides an excellent test framework that makes writing test code and
    96  checking coverage statistics straight forward.  For more information about the
    97  test coverage tools, see the [golang cover blog post](http://blog.golang.org/cover).
    98  
    99  A quick summary of test practices follows:
   100  
   101  - All new code should be accompanied by tests that ensure the code behaves
   102    correctly when given expected values, and, perhaps even more importantly, that
   103    it handles errors gracefully
   104  - When you fix a bug, it should be accompanied by tests which exercise the bug
   105    to both prove it has been resolved and to prevent future regressions
   106  
   107  ## Code Documentation and Commenting
   108  
   109  - At a minimum every function must be commented with its intended purpose and
   110    any assumptions that it makes
   111    - Function comments must always begin with the name of the function per
   112      [Effective Go](http://golang.org/doc/effective_go.html)
   113    - Function comments should be complete sentences since they allow a wide
   114      variety of automated presentations such as [go.dev](https://go.dev)
   115    - The general rule of thumb is to look at it as if you were completely
   116      unfamiliar with the code and ask yourself, would this give me enough
   117      information to understand what this function does and how I'd probably want
   118      to use it?
   119  - Exported functions should also include detailed information the caller of the
   120    function will likely need to know and/or understand:
   121  
   122  **WRONG**
   123  
   124  ```Go
   125  // convert a compact uint32 to big.Int
   126  func CompactToBig(compact uint32) *big.Int {
   127  ```
   128  
   129  **RIGHT**
   130  
   131  ```Go
   132  // CompactToBig converts a compact representation of a whole number N to a
   133  // big integer.  The representation is similar to IEEE754 floating point
   134  // numbers.
   135  //
   136  // Like IEEE754 floating point, there are three basic components: the sign,
   137  // the exponent, and the mantissa. They are broken out as follows:
   138  //
   139  //        * the most significant 8 bits represent the unsigned base 256 exponent
   140  //        * bit 23 (the 24th bit) represents the sign bit
   141  //        * the least significant 23 bits represent the mantissa
   142  //
   143  //        -------------------------------------------------
   144  //        |   Exponent     |    Sign    |    Mantissa     |
   145  //        -------------------------------------------------
   146  //        | 8 bits [31-24] | 1 bit [23] | 23 bits [22-00] |
   147  //        -------------------------------------------------
   148  //
   149  // The formula to calculate N is:
   150  //         N = (-1^sign) * mantissa * 256^(exponent-3)
   151  //
   152  // This compact form is only used in bitcoin to encode unsigned 256-bit numbers
   153  // which represent difficulty targets, thus there really is not a need for a
   154  // sign bit, but it is implemented here to stay consistent with bitcoind.
   155  func CompactToBig(compact uint32) *big.Int {
   156  ```
   157  
   158  - Comments in the body of the code are highly encouraged, but they should
   159    explain the intention of the code as opposed to just calling out the
   160    obvious
   161    
   162  **WRONG**
   163  
   164  ```Go
   165  // return err if amt is less than 5460
   166  if amt < 5460 {
   167    return err
   168  }
   169  ```
   170  
   171  **RIGHT**
   172  
   173  ```Go
   174  // Treat transactions with amounts less than the amount which is considered dust
   175  // as non-standard.
   176  if amt < 5460 {
   177    return err
   178  }
   179  ```
   180  
   181  **NOTE:** The above should really use a constant as opposed to a magic number,
   182  but it was left as a magic number to show how much of a difference a good
   183  comment can make.
   184  
   185  ## Model Git Commit Messages
   186  
   187  This project prefers to keep a clean commit history with well-formed commit
   188  messages.  This section illustrates a model commit message and provides a bit
   189  of background for it.  This content was originally created by Tim Pope and made
   190  available on his website, however that website is no longer active, so it is
   191  being provided here.
   192  
   193  Here’s a model Git commit message:
   194  
   195  ```text
   196  Short (50 chars or less) summary of changes
   197  
   198  More detailed explanatory text, if necessary.  Wrap it to about 72
   199  characters or so.  In some contexts, the first line is treated as the
   200  subject of an email and the rest of the text as the body.  The blank
   201  line separating the summary from the body is critical (unless you omit
   202  the body entirely); tools like rebase can get confused if you run the
   203  two together.
   204  
   205  Write your commit message in the present tense: "Fix bug" and not "Fixed
   206  bug."  This convention matches up with commit messages generated by
   207  commands like git merge and git revert.
   208  
   209  Further paragraphs come after blank lines.
   210  
   211  - Bullet points are okay, too
   212  - Typically a hyphen or asterisk is used for the bullet, preceded by a
   213    single space, with blank lines in between, but conventions vary here
   214  - Use a hanging indent
   215  ```
   216  
   217  Prefix the summary with the subsystem/package when possible. Many other
   218  projects make use of the code and this makes it easier for them to tell when
   219  something they're using has changed. Have a look at [past
   220  commits](https://github.com/btcsuite/btcd/commits/master) for examples of
   221  commit messages.
   222  
   223  Here are some of the reasons why wrapping your commit messages to 72 columns is
   224  a good thing.
   225  
   226  - git log doesn’t do any special special wrapping of the commit messages. With
   227    the default pager of less -S, this means your paragraphs flow far off the edge
   228    of the screen, making them difficult to read. On an 80 column terminal, if we
   229    subtract 4 columns for the indent on the left and 4 more for symmetry on the
   230    right, we’re left with 72 columns.
   231  - git format-patch --stdout converts a series of commits to a series of emails,
   232    using the messages for the message body.  Good email netiquette dictates we
   233    wrap our plain text emails such that there’s room for a few levels of nested
   234    reply indicators without overflow in an 80 column terminal.
   235  
   236  ## Code Approval Process
   237  
   238  This section describes the code approval process that is used for code
   239  contributions.  This is how to get your changes into btcd.
   240  
   241  ## Code Review
   242  
   243  All code which is submitted will need to be reviewed before inclusion into the
   244  master branch.  This process is performed by the project maintainers and usually
   245  other committers who are interested in the area you are working in as well.
   246  
   247  ## Code Review Timeframe
   248  
   249  The timeframe for a code review will vary greatly depending on factors such as
   250  the number of other pull requests which need to be reviewed, the size and
   251  complexity of the contribution, how well you followed the guidelines presented
   252  on this page, and how easy it is for the reviewers to digest your commits.  For
   253  example, if you make one monolithic commit that makes sweeping changes to things
   254  in multiple subsystems, it will obviously take much longer to review.  You will
   255  also likely be asked to split the commit into several smaller, and hence more
   256  manageable, commits.
   257  
   258  Keeping the above in mind, most small changes will be reviewed within a few
   259  days, while large or far reaching changes may take weeks.  This is a good reason
   260  to stick with the [Share Early, Share Often](#ShareOften) development practice
   261  outlined above.
   262  
   263  ## What is the review looking for?
   264  
   265  The review is mainly ensuring the code follows the [Development Practices](#DevelopmentPractices)
   266  and [Code Contribution Standards](#Standards).  However, there are a few other
   267  checks which are generally performed as follows:
   268  
   269  - The code is stable and has no stability or security concerns
   270  - The code is properly using existing APIs and generally fits well into the
   271    overall architecture
   272  - The change is not something which is deemed inappropriate by community
   273    consensus
   274  
   275  ## Rework Code (if needed)
   276  
   277  After the code review, the change will be accepted immediately if no issues are
   278  found.  If there are any concerns or questions, you will be provided with
   279  feedback along with the next steps needed to get your contribution merged with
   280  master.  In certain cases the code reviewer(s) or interested committers may help
   281  you rework the code, but generally you will simply be given feedback for you to
   282  make the necessary changes.
   283  
   284  This process will continue until the code is finally accepted.
   285  
   286  ## Acceptance
   287  
   288  Once your code is accepted, it will be integrated with the master branch.
   289  Typically it will be rebased and fast-forward merged to master as we prefer to
   290  keep a clean commit history over a tangled weave of merge commits.  However,
   291  regardless of the specific merge method used, the code will be integrated with
   292  the master branch and the pull request will be closed.
   293  
   294  Rejoice as you will now be listed as a [contributor](https://github.com/btcsuite/btcd/graphs/contributors)!
   295  
   296  ## Contribution Standards
   297  
   298  ## Contribution Checklist
   299  
   300  - [&nbsp;&nbsp;] All changes are Go version 1.3 compliant
   301  - [&nbsp;&nbsp;] The code being submitted is commented according to the
   302    [Code Documentation and Commenting](#CodeDocumentation) section
   303  - [&nbsp;&nbsp;] For new code: Code is accompanied by tests which exercise both
   304    the positive and negative (error paths) conditions (if applicable)
   305  - [&nbsp;&nbsp;] For bug fixes: Code is accompanied by new tests which trigger
   306    the bug being fixed to prevent regressions
   307  - [&nbsp;&nbsp;] Any new logging statements use an appropriate subsystem and
   308    logging level
   309  - [&nbsp;&nbsp;] Code has been formatted with `go fmt`
   310  - [&nbsp;&nbsp;] Running `go test` does not fail any tests
   311  - [&nbsp;&nbsp;] Running `go vet` does not report any issues
   312  - [&nbsp;&nbsp;] Running [golint](https://github.com/golang/lint) does not
   313    report any **new** issues that did not already exist
   314  
   315  ## Licensing of Contributions
   316  
   317  All contributions must be licensed with the
   318  [ISC license](https://github.com/btcsuite/btcd/blob/master/LICENSE).  This is
   319  the same license as all of the code in the btcd suite.