From 73cd4458c3caab9e723e85af9441002c08c986c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Sat, 25 Jul 2015 12:22:39 +0200 Subject: Update the CONTRIBUTING file Looks like I pushed too early in the previous commit. Oops! --- CONTRIBUTING.asciidoc | 389 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 267 insertions(+), 122 deletions(-) (limited to 'CONTRIBUTING.asciidoc') diff --git a/CONTRIBUTING.asciidoc b/CONTRIBUTING.asciidoc index 7bc005b..d58eb0f 100644 --- a/CONTRIBUTING.asciidoc +++ b/CONTRIBUTING.asciidoc @@ -1,173 +1,318 @@ -Contributing -============ += Contributing -Introduction ------------- +This document is a guide on how to best contribute to this project. -This document describes the usages and rules to follow when contributing -to this project. +== Definitions -It uses the uppercase keywords SHOULD for optional but highly recommended -conditions and MUST for required conditions. +*SHOULD* describes optional steps. *MUST* describes mandatory steps. -`git` is a distributed source code versioning system. This document refers -to three different repositories hosting the source code of the project. -`Your local copy` refers to the copy of the repository that you have on -your computer. The remote repository `origin` refers to your fork of the -project's repository that you can find in your GitHub account. The remote -repository `upstream` refers to the official repository for this project. +*SHOULD NOT* and *MUST NOT* describes pitfalls to avoid. -Following this document will ensure prompt merging of your work in the -`master` branch of the project. +_Your local copy_ refers to the copy of the repository that you have +on your computer. _origin_ refers to your fork of the project. _upstream_ +refers to the official repository for this project. -Reporting bugs --------------- +== Discussions -Upon identifying a bug or a DoS vulnerability, you SHOULD submit a ticket, -regardless of your plans for fixing it. If you plan to fix the bug, you -SHOULD discuss your plans to avoid having your work rejected. +For general discussion about this project, please open a ticket. +Feedback is always welcome and may transform in tasks to improve +the project, so having the discussion start there is a plus. -Upon identifying a security vulnerability in Erlang/OTP that leaves Cowboy -vulnerable to attack, you SHOULD consult privately with the Erlang/OTP team -to get the issue resolved. +Alternatively you may try the #ninenines IRC channel on Freenode, +or, if you need the discussion to stay private, you can send an +email at contact@ninenines.eu. -Upon identifying a security vulnerability in Cowboy's `cowboy_static` module, -you SHOULD submit a ticket, regardless of your plans for fixing it. Please -ensure that all necessary details to reproduce are listed. You then SHOULD -inform users on the mailing list about the issue, advising that they use -another means for sending static files until the issue is resolved. +== Support -Upon identifying a security vulnerability in any other part of Cowboy, you -SHOULD contact us directly by email. Please ensure that all necessary details -to reproduce are listed. +Free support is generally not available. The rule is that free +support is only given if doing so benefits most users. In practice +this means that free support will only be given if the issues are +due to a fault in the project itself or its documentation. -Before implementing a new feature, you SHOULD submit a ticket for discussion -on your plans. The feature might have been rejected already, or the -implementation might already be decided. +Paid support is available for all price ranges. Please send an +email to contact@ninenines.eu for more information. -Cloning -------- +== Bug reports -You MUST fork the project's repository to your GitHub account by clicking -on the `Fork` button. +You *SHOULD* open a ticket for every bug you encounter, regardless +of the version you use. A ticket not only helps the project ensure +that bugs are squashed, it also helps other users who later run +into this issue. -Then, from your fork's page, copy the `Git Read-Only` URL to your clipboard. -You MUST perform the following commands in the folder you choose, replacing -`$URL` by the URL you just copied, `$UPSTREAM_URL` by the `Git Read-Only` -project of the official repository, and `$PROJECT` by the name of this project. +You *SHOULD NOT* open a ticket if another already exists for the +same issue. You *SHOULD* instead either add more information by +commenting on it, or simply comment to inform the maintainer that +you are also affected. The maintainer *SHOULD* reply to every +new ticket when they are opened. If the maintainer didn't say +anything after a few days, you *SHOULD* write a new comment asking +for more information. -``` bash -$ git clone "$URL" +When you have a fix ready, you *SHOULD* open a pull request, +even if the code does not fit the requirements discussed below. +Providing a fix, even a dirty one, can help other users and/or +at least get the maintainer on the right tracks. + +== Security reports + +You *SHOULD* open a ticket when you identify a DoS vulnerability +in this project. You *SHOULD* include the resources needed to +DoS the project; every project can be brought down if you have +the necessary resources. + +You *SHOULD* send an email to contact@ninenines.eu when you +identify a security vulnerability. If the vulnerability originates +from code inside Erlang/OTP itself, you *SHOULD* also consult +with OTP Team directly to get the problem fixed upstream. + +== Feature requests + +Feature requests are always welcome. To be accepted, however, they +must be well defined, make sense in the context of the project and +benefit most users. + +Feature requests not benefiting most users may only be accepted +when accompanied with a proper pull request. + +You *MUST* open a ticket to explain what the new feature is, even +if you are going to submit a pull request for it. + +All these conditions are meant to ensure that the project stays +lightweight and maintainable. + +== Documentation submissions + +You *SHOULD* follow the code submission guidelines to submit +documentation. + +The documentation is available in the 'doc/src/' directory. There +are three kinds of documentation: manual, guide and tutorials. The +format for the documentation is Asciidoc. + +You *SHOULD* follow the same style as the surrounding documentation +when editing existing files. + +You *MUST* include the source when providing media. + +== Examples submissions + +You *SHOULD* follow the code submission guidelines to submit examples. + +The examples are available in the 'examples/' directory. + +You *SHOULD* focus on exactly one thing per example. + +== Code submissions + +You *SHOULD* open a pull request to submit code. + +You *SHOULD* open a ticket to discuss backward incompatible changes +before you submit code. This step ensures that you do not work on +a large change that will then be rejected. + +You *SHOULD* send your code submission using a pull request on GitHub. +If you can't, please send an email to contact@ninenines.eu with your +patch. + +The following sections explain the normal GitHub workflow. + +=== Cloning + +You *MUST* fork the project's repository on GitHub by clicking on the +_Fork_ button. + +On the right page of your fork's page is a field named _SSH clone URL_. +Its contents will be identified as `$ORIGIN_URL` in the following snippet. + +On the right side of the project's repository page is a similar field. +Its contents will be identified as `$UPSTREAM_URL`. + +Finally, `$PROJECT` is the name of this project. + +To setup your clone and be able to rebase when requested, run the +following commands: + +[source,bash] +$ git clone $ORIGIN_URL $ cd $PROJECT $ git remote add upstream $UPSTREAM_URL -``` -Branching ---------- +=== Branching -Before starting working on the code, you MUST update to `upstream`. The -project is always evolving, and as such you SHOULD always strive to keep -up to date when submitting patches to make sure they can be merged without -conflicts. +You *SHOULD* base your branch on _master_, unless your patch applies +to a stable release, in which case you need to base your branch on +the stable branch, for example _1.0.x_. -To update the current branch to `upstream`, you can use the following commands. +The first step is therefore to checkout the branch in question: -``` bash +[source,bash] +$ git checkout 1.0.x + +The next step is to update the branch to the current version from +_upstream_. In the following snippet, replace _1.0.x_ by _master_ +if you are patching _master_. + +[source,bash] $ git fetch upstream -$ git rebase upstream/master -``` +$ git rebase upstream/1.0.x -It may ask you to stash your changes, in which case you stash with: +This last command may fail and ask you to stash your changes. When +that happens, run the following sequence of commands: -``` bash +[source,bash] $ git stash -``` +$ git rebase upstream/1.0.x +$ git stash pop -And put your changes back in with: +The final step is to create a new branch you can work in. The name +of the new branch is up to you, there is no particular requirement. +Replace `$BRANCH` with the branch name you came up with: -``` bash -$ git stash pop -``` +[source,bash] +$ git checkout -b $BRANCH -You SHOULD use these commands both before working on your patch and before -submitting the pull request. If conflicts arise it is your responsability -to deal with them. +_Your local copy_ is now ready. -You MUST create a new branch for your work. First, ensure you are on `master`. -You MUST update `master` to `upstream` before doing anything. Then create a -new branch `$BRANCH` and switch to it. +=== Source editing -``` bash -$ git checkout -b $BRANCH -``` +There are very few rules with regard to source code editing. + +You *MUST* use horizontal tabs for indentation. Use one tab +per indentation level. + +You *MUST NOT* align code. You can only add or remove one +indentation level compared to the previous line. + +You *SHOULD NOT* write lines more than about a hundred +characters. There is no hard limit, just try to keep it +as readable as possible. + +You *SHOULD* write small functions when possible. + +You *SHOULD* avoid a too big hierarchy of case clauses inside +a single function. + +You *SHOULD* add tests to make sure your code works. + +=== Committing + +You *SHOULD* run Dialyzer and the test suite while working on +your patch, and you *SHOULD* ensure that no additional tests +fail when you finish. + +You can use the following command to run Dialyzer: -You MUST use a an insightful branch name. +[source,bash] +$ make dialyze -If you later need to switch back to an existing branch `$BRANCH`, you can use: +You have two options to run tests. You can either run tests +across all supported Erlang versions, or just on the version +you are currently using. -``` bash -$ git checkout $BRANCH -``` +To test across all supported Erlang versions: -Source editing --------------- +[source,bash] +$ make -k ci -The following rules MUST be followed: - * Indentation uses horizontal tabs (1 tab = 4 columns) - * Do NOT align code; only indentation is allowed - * Lines MUST NOT span more than 80 columns +To test using the current version: -The following rules SHOULD be followed: - * Write small functions whenever possible - * Avoid having too many clauses containing clauses containing clauses +[source,bash] +$ make tests -Committing ----------- +You can then open Common Test logs in 'logs/all_runs.html'. -You MUST ensure that all commits pass all tests and do not have extra -Dialyzer warnings. +Once all tests pass (or at least, no new tests are failing), +you can commit your changes. -Running tests is fairly straightforward. Note that you need at least -Erlang/OTP R16B01 for the SSL tests to run. +First you need to add your changes: -``` bash -make tests -``` +[source,bash] +$ git add src/file_you_edited.erl -Running Dialyzer requires some initial setup. You need to build the PLT -file that Dialyzer will use for its analysis. This is a one-time operation. -Dialyzer will take care of updating that file when needed. +If you want an interactive session, allowing you to filter +out changes that have nothing to do with this commit: -``` bash -make build-plt -``` +[source,bash] +$ git add -p -Once that is done, you can run Dialyzer. +You *MUST* put all related changes inside a single commit. The +general rule is that all commits must pass tests. Fix one bug +per commit. Add one feature per commit. Separate features in +multiple commits only if smaller parts of the feature make +sense on their own. -``` bash -make dialyze -``` +Finally once all changes are added you can commit. This +command will open the editor of your choice where you can +put a proper commit title and message. -You MUST put all the related work in a single commit. Fixing a bug is one -commit, adding a feature is one commit, adding two features is two commits. +[source,bash] +$ git commit -You MUST write a proper commit title and message. The commit title MUST be -at most 72 characters; it is the first line of the commit text. The second -line of the commit text MUST be left blank. The third line and beyond is the -commit message. You SHOULD write a commit message. If you do, you MUST make -all lines smaller than 80 characters. You SHOULD explain what the commit -does, what references you used and any other information that helps -understanding your work. +Do not use the `-m` option as it makes it easy to break the +following rules: -Submitting the pull request ---------------------------- +You *MUST* write a proper commit title and message. The commit +title is the first line and *MUST* be at most 72 characters. +The second line *MUST* be left blank. Everything after that is +the commit message. You *SHOULD* write a detailed commit +message. The lines of the message *MUST* be at most 80 characters. +You *SHOULD* explain what the commit does, what references you +used and any other information that helps understanding why +this commit exists. You *MUST NOT* include commands to close +GitHub tickets automatically. -You MUST push your branch `$BRANCH` to GitHub, using the following command: +=== Cleaning the commit history -``` bash +If you create a new commit every time you make a change, however +insignificant, you *MUST* consolidate those commits before +sending the pull request. + +This is done through _rebasing_. The easiest way to do so is +to use interactive rebasing, which allows you to choose which +commits to keep, squash, edit and so on. To rebase, you need +to give the original commit before you made your changes. If +you only did two changes, you can use the shortcut form `HEAD^^`: + +[source,bash] +$ git rebase -i HEAD^^ + +=== Submitting the pull request + +You *MUST* push your branch to your fork on GitHub. Replace +`$BRANCH` with your branch name: + +[source,bash] $ git push origin $BRANCH -``` -You MUST then submit the pull request by using the GitHub interface. -You SHOULD provide an explanatory message and refer to any previous ticket -related to this patch. +You can then submit the pull request using the GitHub interface. +You *SHOULD* provide an explanatory message and refer to any +previous ticket related to this patch. You *MUST NOT* include +commands to close other tickets automatically. + +=== Updating the pull request + +Sometimes the maintainer will ask you to change a few things. +Other times you will notice problems with your submission and +want to fix them on your own. + +In either case you do not need to close the pull request. You +can just push your changes again and, if needed, force them. +This will update the pull request automatically. + +[source,bash] +$ git push -f origin $BRANCH + +=== Merging + +This is an open source project maintained by independent developers. +Please be patient when your changes aren't merged immediately. + +All pull requests run through a Continuous Integration service +to ensure nothing gets broken by the changes submitted. + +Bug fixes will be merged immediately when all tests pass. +The maintainer may do style changes in the merge commit if +the submitter is not available. The maintainer *MUST* open +a new ticket if the solution could still be improved. + +New features and backward incompatible changes will be merged +when all tests pass and all other requirements are fulfilled. -- cgit v1.2.3