From 7a6d9695377a8c814c185d5f70efc99050485bad Mon Sep 17 00:00:00 2001 From: muxator Date: Wed, 15 Aug 2018 02:19:05 +0200 Subject: [PATCH] docs: started writing down the requirements for pull requests No more merge commits in the PR: we want to be able to bisect easily. Probably the whole doc needs to be updated. Also, we need to have templates for PRs and Bug Reports, and they have to be described in the document. Fixes #3454 --- CONTRIBUTING.md | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b6dbe203..a734643b7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,34 @@ # Contributor Guidelines (Please talk to people on the mailing list before you change this page, see our section on [how to get in touch](https://github.com/ether/etherpad-lite#get-in-touch)) +## Pull requests + +* the commit series in the PR should be _linear_ (it **should not contain merge commits**). This is necessary because we want to be able to [bisect](https://en.wikipedia.org/wiki/Bisection_(software_engineering)) bugs easily. Rewrite history/perform a rebase if necessary +* PRs should be issued against the **develop** branch: we never pull directly into **master** +* PRs **should not have conflicts** with develop. If there are, please resolve them rebasing and force-pushing +* when preparing your PR, please make sure that you have included the relevant **changes to the documentation** (preferably with usage examples) +* contain meaningful and detailed **commit messages** in the form: + ``` + submodule: description + + longer description of the change you have made, eventually mentioning the + number of the issue that is being fixed, in the form: Fixes #someIssueNumber + ``` +* if the PR is a **bug fix**: + * the first commit in the series must be a test that shows the failure + * subsequent commits will fix the bug and make the test pass + * the final commit message should include the text `Fixes: #xxx` to link it to its bug report +* think about stability: code has to be backwards compatible as much as possible. Always **assume your code will be run with an older version of the DB/config file** +* if you want to remove a feature, **deprecate it instead**: + * write an issue with your deprecation plan + * output a `WARN` in the log informing that the feature is going to be removed + * remove the feature in the next version +* if you want to add a new feature, put it under a **feature flag**: + * once the new feature has reached a minimal level of stability, do a PR for it, so it can be integrated early + * expose a mechanism for enabling/disabling the feature + * the new feature should be **disabled** by default. With the feature disabled, the code path should be exactly the same as before your contribution. This is a __necessary condition__ for early integration +* think of the PR not as something that __you wrote__, but as something that __someone else is going to read__. The commit series in the PR should tell a novice developer the story of your thoughts when developing it + ## How to write a bug report * Please be polite, we all are humans and problems can occur. @@ -25,12 +53,6 @@ If you send logfiles, please set the loglevel switch DEBUG in your settings.json The logfile location is defined in startup script or the log is directly shown in the commandline after you have started etherpad. - -## Important note for pull requests -**Pull requests should be issued against the develop branch**. We never pull directly into master. - -**Our goal is to iterate in small steps. Release often, release early. Evolution instead of a revolution** - ## General goals of Etherpad To make sure everybody is going in the same direction: * easy to install for admins and easy to use for people