|
| 1 | +# Apollo Contributor Guide |
| 2 | + |
| 3 | +Excited about Apollo and want to make it better? We’re excited too! |
| 4 | + |
| 5 | +Apollo is a community of developers just like you, striving to create the best |
| 6 | +tools and libraries around GraphQL. We welcome anyone who wants to contribute or |
| 7 | +provide constructive feedback, no matter the age or level of experience. If you |
| 8 | +want to help but don't know where to start, let us know, and we'll find |
| 9 | +something for you. |
| 10 | + |
| 11 | +Oh, and if you haven't already, sign up for the |
| 12 | +[Apollo Slack](http://www.apollodata.com/#slack). |
| 13 | + |
| 14 | +Here are some ways to contribute to the project, from easiest to most difficult: |
| 15 | + |
| 16 | +* [Reporting bugs](#reporting-bugs) |
| 17 | +* [Improving the documentation](#improving-the-documentation) |
| 18 | +* [Responding to issues](#responding-to-issues) |
| 19 | +* [Small bug fixes](#small-bug-fixes) |
| 20 | +* [Suggesting features](#feature-requests) |
| 21 | +* [Big pull requests](#big-prs) |
| 22 | + |
| 23 | +## Issues |
| 24 | + |
| 25 | +### Reporting bugs |
| 26 | + |
| 27 | +If you encounter a bug, please file an issue on GitHub via the repository of the |
| 28 | +sub-project you think contains the bug. If an issue you have is already |
| 29 | +reported, please add additional information or add a 👍 reaction to indicate |
| 30 | +your agreement. |
| 31 | + |
| 32 | +While we will try to be as helpful as we can on any issue reported, please |
| 33 | +include the following to maximize the chances of a quick fix: |
| 34 | + |
| 35 | +1. **Intended outcome:** What you were trying to accomplish when the bug |
| 36 | + occurred, and as much code as possible related to the source of the problem. |
| 37 | +2. **Actual outcome:** A description of what actually happened, including a |
| 38 | + screenshot or copy-paste of any related error messages, logs, or other output |
| 39 | + that might be related. Places to look for information include your browser |
| 40 | + console, server console, and network logs. Please avoid non-specific phrases |
| 41 | + like “didn’t work” or “broke”. |
| 42 | +3. **How to reproduce the issue:** Instructions for how the issue can be |
| 43 | + reproduced by a maintainer or contributor. Be as specific as possible, and |
| 44 | + only mention what is necessary to reproduce the bug. If possible, build a |
| 45 | + reproduction with our |
| 46 | + [error template](https://github.com/apollographql/react-apollo-error-template) |
| 47 | + to isolate the exact circumstances in which the bug occurs. Avoid speculation |
| 48 | + over what the cause might be. |
| 49 | + |
| 50 | +Creating a good reproduction really helps contributors investigate and resolve |
| 51 | +your issue quickly. In many cases, the act of creating a minimal reproduction |
| 52 | +illuminates that the source of the bug was somewhere outside the library in |
| 53 | +question, saving time and effort for everyone. |
| 54 | + |
| 55 | +### Improving the documentation |
| 56 | + |
| 57 | +Improving the documentation, examples, and other open source content can be the |
| 58 | +easiest way to contribute to the library. If you see a piece of content that can |
| 59 | +be better, open a PR with an improvement, no matter how small! If you would like |
| 60 | +to suggest a big change or major rewrite, we’d love to hear your ideas but |
| 61 | +please open an issue for discussion before writing the PR. |
| 62 | + |
| 63 | +### Responding to issues |
| 64 | + |
| 65 | +In addition to reporting issues, a great way to contribute to Apollo is to |
| 66 | +respond to other peoples' issues and try to identify the problem or help them |
| 67 | +work around it. If you’re interested in taking a more active role in this |
| 68 | +process, please go ahead and respond to issues. And don't forget to say "Hi" on |
| 69 | +Apollo Slack! |
| 70 | + |
| 71 | +### Small bug fixes |
| 72 | + |
| 73 | +For a small bug fix change (less than 20 lines of code changed), feel free to |
| 74 | +open a pull request. We’ll try to merge it as fast as possible and ideally |
| 75 | +publish a new release on the same day. The only requirement is, make sure you |
| 76 | +also add a test that verifies the bug you are trying to fix. |
| 77 | + |
| 78 | +### Suggesting features |
| 79 | + |
| 80 | +Most of the features in Apollo came from suggestions by you, the community! We |
| 81 | +welcome any ideas about how to make Apollo better for your use case. Unless |
| 82 | +there is overwhelming demand for a feature, it might not get implemented |
| 83 | +immediately, but please include as much information as possible that will help |
| 84 | +people have a discussion about your proposal: |
| 85 | + |
| 86 | +1. **Use case:** What are you trying to accomplish, in specific terms? Often, |
| 87 | + there might already be a good way to do what you need and a new feature is |
| 88 | + unnecessary, but it’s hard to know without information about the specific use |
| 89 | + case. |
| 90 | +2. **Could this be a plugin?** In many cases, a feature might be too niche to be |
| 91 | + included in the core of a library, and is better implemented as a companion |
| 92 | + package. If there isn’t a way to extend the library to do what you want, |
| 93 | + could we add additional plugin APIs? It’s important to make the case for why |
| 94 | + a feature should be part of the core functionality of the library. |
| 95 | +3. **Is there a workaround?** Is this a more convenient way to do something that |
| 96 | + is already possible, or is there some blocker that makes a workaround |
| 97 | + unfeasible? |
| 98 | + |
| 99 | +Feature requests will be labeled as such, and we encourage using GitHub issues |
| 100 | +as a place to discuss new features and possible implementation designs. Please |
| 101 | +refrain from submitting a pull request to implement a proposed feature until |
| 102 | +there is consensus that it should be included. This way, you can avoid putting |
| 103 | +in work that can’t be merged in. |
| 104 | + |
| 105 | +Once there is a consensus on the need for a new feature, proceed as listed below |
| 106 | +under “Big PRs”. |
| 107 | + |
| 108 | +## Big PRs |
| 109 | + |
| 110 | +This includes: |
| 111 | + |
| 112 | +* Big bug fixes |
| 113 | +* New features |
| 114 | + |
| 115 | +For significant changes to a repository, it’s important to settle on a design |
| 116 | +before starting on the implementation. This way, we can make sure that major |
| 117 | +improvements get the care and attention they deserve. Since big changes can be |
| 118 | +risky and might not always get merged, it’s good to reduce the amount of |
| 119 | +possible wasted effort by agreeing on an implementation design/plan first. |
| 120 | + |
| 121 | +1. **Open an issue.** Open an issue about your bug or feature, as described |
| 122 | + above. |
| 123 | +2. **Reach consensus.** Some contributors and community members should reach an |
| 124 | + agreement that this feature or bug is important, and that someone should work |
| 125 | + on implementing or fixing it. |
| 126 | +3. **Agree on intended behavior.** On the issue, reach an agreement about the |
| 127 | + desired behavior. In the case of a bug fix, it should be clear what it means |
| 128 | + for the bug to be fixed, and in the case of a feature, it should be clear |
| 129 | + what it will be like for developers to use the new feature. |
| 130 | +4. **Agree on implementation plan.** Write a plan for how this feature or bug |
| 131 | + fix should be implemented. What modules need to be added or rewritten? Should |
| 132 | + this be one pull request or multiple incremental improvements? Who is going |
| 133 | + to do each part? |
| 134 | +5. **Submit PR.** In the case where multiple dependent patches need to be made |
| 135 | + to implement the change, only submit one at a time. Otherwise, the others |
| 136 | + might get stale while the first is reviewed and merged. Make sure to avoid |
| 137 | + “while we’re here” type changes - if something isn’t relevant to the |
| 138 | + improvement at hand, it should be in a separate PR; this especially includes |
| 139 | + code style changes of unrelated code. |
| 140 | +6. **Review.** At least one core contributor should sign off on the change |
| 141 | + before it’s merged. Look at the “code review” section below to learn about |
| 142 | + factors are important in the code review. If you want to expedite the code |
| 143 | + being merged, try to review your own code first! |
| 144 | +7. **Merge and release!** |
| 145 | + |
| 146 | +### Code review guidelines |
| 147 | + |
| 148 | +It’s important that every piece of code in Apollo packages is reviewed by at |
| 149 | +least one core contributor familiar with that codebase. Here are some things we |
| 150 | +look for: |
| 151 | + |
| 152 | +1. **Required CI checks pass.** This is a prerequisite for the review, and it is |
| 153 | + the PR author's responsibility. As long as the tests don’t pass, the PR won't |
| 154 | + get reviewed. |
| 155 | +2. **Simplicity.** Is this the simplest way to achieve the intended goal? If |
| 156 | + there are too many files, redundant functions, or complex lines of code, |
| 157 | + suggest a simpler way to do the same thing. In particular, avoid implementing |
| 158 | + an overly general solution when a simple, small, and pragmatic fix will do. |
| 159 | +3. **Testing.** Do the tests ensure this code won’t break when other stuff |
| 160 | + changes around it? When it does break, will the tests added help us identify |
| 161 | + which part of the library has the problem? Did we cover an appropriate set of |
| 162 | + edge cases? Look at the test coverage report if there is one. Are all |
| 163 | + significant code paths in the new code exercised at least once? |
| 164 | +4. **No unnecessary or unrelated changes.** PRs shouldn’t come with random |
| 165 | + formatting changes, especially in unrelated parts of the code. If there is |
| 166 | + some refactoring that needs to be done, it should be in a separate PR from a |
| 167 | + bug fix or feature, if possible. |
| 168 | +5. **Code has appropriate comments.** Code should be commented, or written in a |
| 169 | + clear “self-documenting” way. |
| 170 | +6. **Idiomatic use of the language.** In TypeScript, make sure the typings are |
| 171 | + specific and correct. In ES2015, make sure to use imports rather than require |
| 172 | + and const instead of var, etc. Ideally a linter enforces a lot of this, but |
| 173 | + use your common sense and follow the style of the surrounding code. |
0 commit comments