From b522c93f7a36af33b97b7022e75efa66a219735f Mon Sep 17 00:00:00 2001 From: cevap Date: Fri, 7 Dec 2018 01:31:17 +0100 Subject: [PATCH] Update CONTRIBUTING.md - Add Table of Contents --- CONTRIBUTING.md | 160 +++++++++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 75 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ba4653e93..536e257a8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,20 @@ -Contributing to Ion Core -============================ +# Contributing to Ion Core + +Table of Contents +----------------- + +- [Contributing to Ion Core](#contributing-to-ion-core) + - [Contributor Workflow](#contributor-workflow) + - [Examples:](#examples) + - [Squashing Commits](#squashing-commits) + - [Pull Request Philosophy](#pull-request-philosophy) + - [Features](#features) + - [Refactoring](#refactoring) + - ["Decision Making" Process](#%22decision-making%22-process) + - [Peer Review](#peer-review) + - [Finding Reviewers](#finding-reviewers) + - [Release Policy](#release-policy) + - [Copyright](#copyright) The Ion Core project operates an open contributor model where anyone is welcome to contribute towards development in the form of peer review, testing @@ -15,8 +30,7 @@ merging pull requests as well as a "lead maintainer" who is responsible for the release cycle, overall merging, moderation and appointment of maintainers. -Contributor Workflow --------------------- +## Contributor Workflow The codebase is maintained using the "contributor workflow" where everyone without exception contributes patch proposals using "pull requests". This @@ -24,9 +38,9 @@ facilitates social contribution, easy testing and peer review. To contribute a patch, the workflow is as follows: - - Fork repository - - Create topic branch - - Commit patches + 1. Fork repository + 2. Create topic branch + 3. Commit patches The project coding conventions in the [developer notes](doc/developer-notes.md) must be adhered to. @@ -36,7 +50,7 @@ and diffs should be easy to read. For this reason do not mix any formatting fixes or code moves with actual code changes. Commit messages should be verbose by default consisting of a short subject line -(50 chars max), a blank line and detailed explanatory text as separate +(**50 chars max**), a blank line and detailed explanatory text as separate paragraph(s), unless the title alone is self-explanatory (like "Corrected typo in init.cpp") in which case a single title line is sufficient. Commit messages should be helpful to people reading your code in the future, so explain the reasoning for @@ -55,26 +69,26 @@ about Git. The title of the pull request should be prefixed by the component or area that the pull request affects. Valid areas as: - - *Consensus* for changes to consensus critical code - - *Docs* for changes to the documentation - - *Qt* for changes to ion-qt - - *Minting* for changes to the minting code - - *Net* or *P2P* for changes to the peer-to-peer network code - - *RPC/REST* for changes to the RPC or REST APIs - - *Scripts and tools* for changes to the scripts and tools - - *Tests* for changes to the ion unit tests or QA tests - - *Trivial* should **only** be used for PRs that do not change generated - executable code. Notably, refactors (change of function arguments and code - reorganization) and changes in behavior should **not** be marked as trivial. - Examples of trivial PRs are changes to: - - comments - - whitespace - - variable names - - logging and messages - - *Utils and libraries* for changes to the utils and libraries - - *Wallet* for changes to the wallet code - -Examples: +- *Consensus* for changes to consensus critical code +- *Docs* for changes to the documentation +- *Qt* for changes to ion-qt +- *Minting* for changes to the minting code +- *Net* or *P2P* for changes to the peer-to-peer network code +- *RPC/REST* for changes to the RPC or REST APIs +- *Scripts and tools* for changes to the scripts and tools +- *Tests* for changes to the ion unit tests or QA tests +- *Trivial* should **only** be used for PRs that do not change generated + executable code. Notably, refactors (change of function arguments and code + reorganization) and changes in behavior should **not** be marked as trivial. + Examples of trivial PRs are changes to: + - comments + - whitespace + - variable names + - logging and messages +- *Utils and libraries* for changes to the utils and libraries +- *Wallet* for changes to the wallet code + +### Examples: Consensus: Add new opcode for BIP-XXXX OP_CHECKAWESOMESIG Net: Automatically create hidden service, listen on Tor @@ -94,8 +108,8 @@ At this stage one should expect comments and review from other contributors. You can add more commits to your pull request by committing them locally and pushing to your fork until you have satisfied all feedback. -Squashing Commits ---------------------------- +## Squashing Commits + If your pull request is accepted for merging, you may be asked by a maintainer to squash and or [rebase](https://git-scm.com/docs/git-rebase) your commits before it will be merged. The basic squashing workflow is shown below. @@ -121,8 +135,7 @@ The length of time required for peer review is unpredictable and will vary from pull request to pull request. -Pull Request Philosophy ------------------------ +## Pull Request Philosophy Patch sets should always be focused. For example, a pull request could add a feature, fix a bug, or refactor code; but not a mixture. Please also avoid super @@ -154,8 +167,7 @@ Project maintainers aim for a quick turnaround on refactoring pull requests, so where possible keep them short, un-complex and easy to verify. -"Decision Making" Process -------------------------- +## "Decision Making" Process The following applies to code changes to the Ion Core project, and is not to be confused with overall Ion Network Protocol consensus changes. @@ -169,10 +181,10 @@ judge the general consensus of contributors. In general, all pull requests must: - - have a clear use case, fix a demonstrable bug or serve the greater good of - the project (for example refactoring for modularisation); - - be well peer reviewed; - - follow code style guidelines; +- have a clear use case, fix a demonstrable bug or serve the greater good of + the project (for example refactoring for modularisation); +- be well peer reviewed; +- follow code style guidelines; Patches that change Ion consensus rules are considerably more involved than normal because they affect the entire ecosystem and so must be preceded by @@ -191,15 +203,15 @@ consensus to merge a pull request (remember that discussions may have been spread out over GitHub, forums, email, and Slack discussions). The following language is used within pull-request comments: - - ACK means "I have tested the code and I agree it should be merged"; - - NACK means "I disagree this should be merged", and must be accompanied by - sound technical justification (or in certain cases of copyright/patent/licensing - issues, legal justification). NACKs without accompanying reasoning may be - disregarded; - - utACK means "I have not tested the code, but I have reviewed it and it looks - OK, I agree it can be merged"; - - Concept ACK means "I agree in the general principle of this pull request"; - - Nit refers to trivial, often non-blocking issues. +- ACK means "I have tested the code and I agree it should be merged"; +- NACK means "I disagree this should be merged", and must be accompanied by + sound technical justification (or in certain cases of copyright/patent/licensing + issues, legal justification). NACKs without accompanying reasoning may be + disregarded; +- utACK means "I have not tested the code, but I have reviewed it and it looks + OK, I agree it can be merged"; +- Concept ACK means "I agree in the general principle of this pull request"; +- Nit refers to trivial, often non-blocking issues. Reviewers should include the commit hash which they reviewed in their comments. @@ -227,37 +239,35 @@ that you've been waiting for a pull request to be given attention for several months, there may be a number of reasons for this, some of which you can do something about: - - It may be because of a feature freeze due to an upcoming release. During this time, - only bug fixes are taken into consideration. If your pull request is a new feature, - it will not be prioritized until the release is over. Wait for release. - - It may be because the changes you are suggesting do not appeal to people. Rather than - nits and critique, which require effort and means they care enough to spend time on your - contribution, thundering silence is a good sign of widespread (mild) dislike of a given change - (because people don't assume *others* won't actually like the proposal). Don't take - that personally, though! Instead, take another critical look at what you are suggesting - and see if it: changes too much, is too broad, doesn't adhere to the - [developer notes](doc/developer-notes.md), is dangerous or insecure, is messily written, etc. - Identify and address any of the issues you find. Then ask e.g. on Slack if someone could give - their opinion on the concept itself. - - It may be because your code is too complex for all but a few people. And those people - may not have realized your pull request even exists. A great way to find people who - are qualified and care about the code you are touching is the - [Git Blame feature](https://help.github.com/articles/tracing-changes-in-a-file/). Simply - find the person touching the code you are touching before you and see if you can find - them and give them a nudge. Don't be incessant about the nudging though. - - Finally, if all else fails, ask on Slack or elsewhere for someone to give your pull request - a look. If you think you've been waiting an unreasonably long amount of time (month+) for - no particular reason (few lines changed, etc), this is totally fine. Try to return the favor - when someone else is asking for feedback on their code, and universe balances out. - - -Release Policy --------------- +- It may be because of a feature freeze due to an upcoming release. During this time, + only bug fixes are taken into consideration. If your pull request is a new feature, + it will not be prioritized until the release is over. Wait for release. +- It may be because the changes you are suggesting do not appeal to people. Rather than + nits and critique, which require effort and means they care enough to spend time on your + contribution, thundering silence is a good sign of widespread (mild) dislike of a given change + (because people don't assume *others* won't actually like the proposal). Don't take + that personally, though! Instead, take another critical look at what you are suggesting + and see if it: changes too much, is too broad, doesn't adhere to the + [developer notes](doc/developer-notes.md), is dangerous or insecure, is messily written, etc. + Identify and address any of the issues you find. Then ask e.g. on Slack if someone could give + their opinion on the concept itself. +- It may be because your code is too complex for all but a few people. And those people + may not have realized your pull request even exists. A great way to find people who + are qualified and care about the code you are touching is the + [Git Blame feature](https://help.github.com/articles/tracing-changes-in-a-file/). Simply + find the person touching the code you are touching before you and see if you can find + them and give them a nudge. Don't be incessant about the nudging though. +- Finally, if all else fails, ask on Slack or elsewhere for someone to give your pull request + a look. If you think you've been waiting an unreasonably long amount of time (month+) for + no particular reason (few lines changed, etc), this is totally fine. Try to return the favor + when someone else is asking for feedback on their code, and universe balances out. + + +## Release Policy The project leader is the release manager for each Ion Core release. -Copyright ---------- +## Copyright By contributing to this repository, you agree to license your work under the MIT license unless specified otherwise in `contrib/debian/copyright` or at