Commit 4da9843e7fba4eb8e3a63bc1f81d72166bc07f28
Added info on making good pull requests.
robert committed on 1/8/2018, 1:09:20 PMParent: 9ad43e00031a920b00354a039760c5dc21783667
Files changed
| contributing.md | changed |
| contributing.md | |||
|---|---|---|---|
| @@ -63,8 +63,44 @@ | |||
| 63 | 63 … | ### Licenses | |
| 64 | 64 … | ||
| 65 | 65 … | Most code is licensed under a permissive license (MIT / ISC), while other code like Patchwork is licensed under a copyleft license (GPL / AGPL). | |
| 66 | 66 … | ||
| 67 … | +## Making a Good Pull Request | ||
| 68 … | + | ||
| 69 … | +Goal: be as easy (for the maintainer) to merge as possible. (This means it can get merged faster.) | ||
| 70 … | + | ||
| 71 … | +### Explain what you are trying to do | ||
| 72 … | + | ||
| 73 … | +All pull requests should have a description explaining what their goal is. Stating a goal helps the maintainer evaluate whether the proposed change is the simplest way to achieve that goal. If the PR is just code then it is much harder to figure out the original intention of that code. | ||
| 74 … | + | ||
| 75 … | +### Document what sort of change it is (patch, feature, breaking change) | ||
| 76 … | + | ||
| 77 … | +What type of change you are proposing affects what you need to communicate. | ||
| 78 … | + | ||
| 79 … | +The main types of changes are: | ||
| 80 … | +1. Patches | ||
| 81 … | +2. Features | ||
| 82 … | +3. Breaking changes | ||
| 83 … | + | ||
| 84 … | +### Patches | ||
| 85 … | +Bug fixing. Documenting this is easy. Explain the problem you intend to fix, and ideally also add a test case that reproduces the problem you found. | ||
| 86 … | + | ||
| 87 … | +### Features | ||
| 88 … | +Adding new features is more challenging. Now your PR needs to explain the use-case. Consider the following when writing your PR: | ||
| 89 … | +- Why do you want this feature? | ||
| 90 … | +- What problem are you trying to solve with it? | ||
| 91 … | + | ||
| 92 … | +When you make a pull request you are not just making a change to the code, you are also making a request to the maintainer: you are asking that they acknowledge this change and give this use their blessing. | ||
| 93 … | + | ||
| 94 … | +When possible, new modules are better than a PR, and then you can ask the maintainer to link to your module's README from their README. | ||
| 95 … | + | ||
| 96 … | +Code style is considered but not usually a deal breaker for a PR, since code style change always change later. However, the API style is very important because breaking changes require changes throughout dependent modules. | ||
| 97 … | + | ||
| 98 … | +For some really popular modules that a lot of code depends on, new features will require a very significant justification of why they are needed. | ||
| 99 … | + | ||
| 100 … | +### Breaking Changes | ||
| 101 … | +Breaking changes are rare. However, if you must make a breaking change, one that restricts the API to a more minmal set is most likely to get merged. Expect a proposed breaking change to get lots of discussion. | ||
| 102 … | + | ||
| 67 | 103 … | ## Contributing Documentation | |
| 68 | 104 … | ||
| 69 | 105 … | While over time we have scattered documentation around: | |
| 70 | 106 … | ||
Built with git-ssb-web