|
| 1 | +# Contribution Guidelines |
| 2 | + |
| 3 | +We welcome many kinds of community contributions to this project! Whether it's a feature implementation, |
| 4 | +bug fix, or a good idea, please create an issue so that we can discuss it. It is not necessary to create an |
| 5 | +issue before sending a pull request but it may speed up the process if we can discuss your idea before |
| 6 | +you start implementing it. |
| 7 | + |
| 8 | +Because this project exposes a couple different public APIs, we must be very mindful of any potential breaking |
| 9 | +changes. Some contributions may not be accepted if they risk introducing breaking changes or if they |
| 10 | +don't match the goals of the project. The core maintainer team has the right of final approval over |
| 11 | +any contribution to this project. However, we are very happy to hear community feedback on any decision |
| 12 | +so that we can ensure we are solving the right problems in the right way. |
| 13 | + |
| 14 | +## Ways to Contribute |
| 15 | + |
| 16 | +- File a bug or feature request as an [issue](https://github.com/PowerShell/PowerShellEditorServices/issues) |
| 17 | +- Comment on existing issues to give your feedback on how they should be fixed/implemented |
| 18 | +- Contribute a bug fix or feature implementation by submitting a pull request |
| 19 | +- Contribute more unit tests for feature areas that lack good coverage |
| 20 | +- Review the pull requests that others submit to ensure they follow [established guidelines] |
| 21 | + (#pull-request-guidelines) |
| 22 | +- Help others gets started with the project by contributing documentation or hanging out |
| 23 | + in the #editors room in the [PowerShell community Slack chat](http://slack.poshcode.org). |
| 24 | + |
| 25 | +## Code Contribution Guidelines |
| 26 | + |
| 27 | +Here's a high level list of guidelines to follow to ensure your code contribution is accepted: |
| 28 | + |
| 29 | +- Follow established guidelines for coding style and design |
| 30 | +- Follow established guidelines for commit hygiene |
| 31 | +- Write unit tests to validate new features and bug fixes |
| 32 | +- Ensure that the 'Release' build and unit tests pass locally |
| 33 | +- Ensure that the AppVeyor build passes for your change |
| 34 | +- Respond to all review feedback and final commit cleanup |
| 35 | + |
| 36 | +### Practice Good Commit Hygiene |
| 37 | + |
| 38 | +First of all, make sure you are practicing [good commit hygiene](http://blog.ericbmerritt.com/2011/09/21/commit-hygiene-and-git.html) |
| 39 | +so that your commits provide a good history of the changes you are making. To be more specific: |
| 40 | + |
| 41 | +- **Write good commit messages** |
| 42 | + |
| 43 | + Commit messages should be clearly written so that a person can look at the commit log and understand |
| 44 | + how and why a given change was made. Here is a great model commit message taken from a [blog post |
| 45 | + by Tim Pope](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html): |
| 46 | + |
| 47 | + Capitalized, short (50 chars or less) summary |
| 48 | + |
| 49 | + More detailed explanatory text, if necessary. Wrap it to about 72 |
| 50 | + characters or so. In some contexts, the first line is treated as the |
| 51 | + subject of an email and the rest of the text as the body. The blank |
| 52 | + line separating the summary from the body is critical (unless you omit |
| 53 | + the body entirely); tools like rebase can get confused if you run the |
| 54 | + two together. |
| 55 | + |
| 56 | + Write your commit message in the imperative: "Fix bug" and not "Fixed bug" |
| 57 | + or "Fixes bug." This convention matches up with commit messages generated |
| 58 | + by commands like git merge and git revert. |
| 59 | + |
| 60 | + Further paragraphs come after blank lines. |
| 61 | + |
| 62 | + - Bullet points are okay, too |
| 63 | + |
| 64 | + - Typically a hyphen or asterisk is used for the bullet, followed by a |
| 65 | + single space, with blank lines in between, but conventions vary here |
| 66 | + |
| 67 | + - Use a hanging indent |
| 68 | + |
| 69 | + A change that fixes a known bug with an issue filed should use the proper syntax so that the [issue |
| 70 | + is automatically closed](https://help.github.com/articles/closing-issues-via-commit-messages/) once |
| 71 | + your change is merged. Here's an example of what such a commit message should look like: |
| 72 | + |
| 73 | + Fix #3: Catch NullReferenceException from DoThing |
| 74 | + |
| 75 | + This change adds a try/catch block to catch a NullReferenceException that |
| 76 | + gets thrown by DoThing [...] |
| 77 | + |
| 78 | +- **Squash your commits** |
| 79 | + |
| 80 | + If you are introducing a new feature but have implemented it over multiple commits, |
| 81 | + please [squash those commits](http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html) |
| 82 | + into a single commit that contains all the changes in one place. This especially applies to any "oops" |
| 83 | + commits where a file is forgotten or a typo is being fixed. Following this approach makes it a lot easier |
| 84 | + to pull those changes to other branches or roll back the change if necessary. |
| 85 | + |
| 86 | +- **Keep individual commits for larger changes** |
| 87 | + |
| 88 | + You can certainly maintain individual commits for different phases of a big change. For example, if |
| 89 | + you want to reorganize some files before adding new functionality, have your first commit contain all |
| 90 | + of the file move changes and then the following commit can have all of the feature additions. We |
| 91 | + highly recommend this approach so that larger commits don't turn into a jumbled mess. |
| 92 | + |
| 93 | +### Add Unit Tests for New Code |
| 94 | + |
| 95 | +If you're adding a new feature to the project, please make sure to include adequate [xUnit](http://xunit.github.io/) |
| 96 | +tests with your change. In this project, we have chosen write out unit tests in a way that uses the |
| 97 | +actual PowerShell environment rather than extensive interface mocking. This allows us to be sure that |
| 98 | +our features will work in practice. |
| 99 | + |
| 100 | +We do both component-level and scenario-level testing depending on what code is being tested. We don't |
| 101 | +expect contributors to test every possible edge case. Testing mainline scenarios and the most common |
| 102 | +failure scenarios is often good enough. |
| 103 | + |
| 104 | +We are very happy to accept unit test contributions for any feature areas that are more error-prone than |
| 105 | +others. Also, if you find that a feature fails for you in a specific case, please feel free to file an issue |
| 106 | +that includes a unit test which reproduces the problem. This will allow us to quickly implement a fix |
| 107 | +that resolves the problem. |
| 108 | + |
| 109 | +### Build 'Release' Before Submitting |
| 110 | + |
| 111 | +Before you send out your pull request, make sure that you have run a Release configuration build of the |
| 112 | +project and that all new and existing tests are passing. The Release configuration build ensures that |
| 113 | +all public API interfaces have been documented correctly otherwise it throws an error. We have turned |
| 114 | +on this check so that our project will always have good generated documentation. |
| 115 | + |
| 116 | +### Follow the Pull Request Process |
| 117 | + |
| 118 | +- **Create your pull request** |
| 119 | + |
| 120 | + Use the [typical process](https://help.github.com/articles/using-pull-requests/) to send a pull request |
| 121 | + from your fork of the project. In your pull request message, please give a high-level summary of the |
| 122 | + changes that you have made so that reviewers understand the intent of the changes. You should receive |
| 123 | + initial comments within a day or two, but please feel free to ping if things are taking longer than |
| 124 | + expected. |
| 125 | + |
| 126 | +- **The build and unit tests must run green** |
| 127 | + |
| 128 | + When you submit your pull request, our automated build system on AppVeyor will attempt to run a |
| 129 | + Release build of your changes and then run all unit tests against the build. If you notice that |
| 130 | + any of your unit tests have failed, please fix them by creating a new commit and then pushing it |
| 131 | + to your branch. If you see that some unrelated test has failed, try re-running the build for your |
| 132 | + pull request. If you continue to see issues, write a comment on the pull request and we will |
| 133 | + look into it. |
| 134 | + |
| 135 | +- **Respond to code review feedback** |
| 136 | + |
| 137 | + If the reviewers ask you to make changes, make them as a new commit to your branch and push them so |
| 138 | + that they are made available for a final review pass. Do not rebase the fixes just yet so that the |
| 139 | + commit hash changes don't upset GitHub's pull request UI. |
| 140 | + |
| 141 | +- **If necessary, do a final rebase** |
| 142 | + |
| 143 | + Once your final changes have been accepted, we may ask you to do a final rebase to have your commits |
| 144 | + so that they follow our commit guidelines. If specific guidance is given, please follow it when |
| 145 | + rebasing your commits. Once you do your final push and we see the AppVeyor build pass, we will |
| 146 | + merge your changes! |
| 147 | + |
0 commit comments