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