UNPKG

11.1 kBMarkdownView Raw
1# Contributing to RxJS
2
3[Read and abide by the Code of Conduct](CODE_OF_CONDUCT.md)! Even if you don't read it,
4it still applies to you. Ignorance is not an exemption.
5
6Contents
7
8<!-- START doctoc generated TOC please keep comment here to allow auto update -->
9<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
10
11
12- [Submitting a Pull Request (PR)](#submitting-a-pull-request-pr)
13 - [After your pull request is merged](#after-your-pull-request-is-merged)
14- [Coding Style Guidelines](#coding-style-guidelines)
15- [Documentation](#documentation)
16- [Unit Tests](#unit-tests)
17 - [CI Tests](#ci-tests)
18- [Performance Tests](#performance-tests)
19 - [Macro](#macro)
20 - [Micro](#micro)
21- [Commit Message Guidelines](#commit-message-guidelines)
22 - [Commit Message Format](#commit-message-format)
23 - [Revert](#revert)
24 - [Type](#type)
25 - [Scope](#scope)
26 - [Subject](#subject)
27 - [Body](#body)
28 - [Footer](#footer)
29
30<!-- END doctoc generated TOC please keep comment here to allow auto update -->
31
32---
33
34- Related documents
35 - [Creating Operators](docs_app/content/guide/operators.md#creating-custom-operators)
36 - [Writing Marble Tests](docs_app/content/guide/testing/marble-testing.md)
37
38---
39
40(This document is a work and progress and is subject to change)
41
42## Submitting a Pull Request (PR)
43
44Before you submit your Pull Request (PR) consider the following guidelines:
45
46- Search [GitHub](https://github.com/ReactiveX/RxJS/pulls) for an open or closed PR
47 that relates to your submission. You don't want to duplicate effort.
48- Make your changes in a new git branch:
49
50 ```shell
51 git checkout -b my-fix-branch master
52 ```
53
54- Create your patch, following [code style guidelines](#coding-style-guidelines), and **including appropriate test cases**.
55- Run the full test suite and ensure that all tests pass.
56- Run the micro and macro performance tests against your feature branch and compare against master
57 to ensure performance wasn't changed for the worse.
58- Commit your changes using a descriptive commit message that follows our
59 [commit message guidelines](#commit-message-guidelines). Adherence to these conventions
60 is necessary because release notes are automatically generated from these messages.
61
62 ```shell
63 git commit -a
64 ```
65
66 Note: the optional commit `-a` command line option will automatically "add" and "rm" edited files.
67
68- Push your branch to GitHub:
69
70 ```shell
71 git push origin my-fix-branch
72 ```
73
74- In GitHub, send a pull request to `RxJS:master`.
75- If we suggest changes then:
76
77 - Make the required updates.
78 - Re-run the test suites to ensure tests are still passing.
79 - Re-run performance tests to make sure your changes didn't hurt performance.
80 - Rebase your branch and force push to your GitHub repository (this will update your Pull Request):
81
82 ```shell
83 git rebase master -i
84 git push -f
85 ```
86
87 - When updating your feature branch with the requested changes, please do not overwrite the commit history, but rather contain the changes in new commits. This is for the sake of a clearer and easier review process.
88
89That's it! Thank you for your contribution!
90
91### After your pull request is merged
92
93After your pull request is merged, you can safely delete your branch and pull the changes
94from the main (upstream) repository:
95
96- Delete the remote branch on GitHub either through the GitHub web UI or your local shell as follows:
97
98 ```shell
99 git push origin --delete my-fix-branch
100 ```
101
102- Check out the master branch:
103
104 ```shell
105 git checkout master -f
106 ```
107
108- Delete the local branch:
109
110 ```shell
111 git branch -D my-fix-branch
112 ```
113
114- Update your master with the latest upstream version:
115
116 ```shell
117 git pull --ff upstream master
118 ```
119
120## Coding Style Guidelines
121
122- Please use proper types and generics throughout your code.
123- 2 space indentation only
124- favor readability over terseness
125
126(TBD): For now try to follow the style that exists elsewhere in the source, and use your best judgment.
127
128## Documentation
129
130- The documentation is auto-generated directly from the source code.
131- In short: From the source code we generate JSON documents, describing each operator, function ... and render this JSON within an Angular application
132- The folder `docs-app` contains everything you need for building and developing the docs
133- The [Documentation README](docs_app/README.md) will support you
134- After a PR is merged to master the docs will be published to https://rxjs.dev/
135
136## Unit Tests
137
138Unit tests are located under the [spec directory](/spec). Unit tests over synchronous operators and operations
139can be written in a standard [chai](https://www.chaijs.com/) style. Unit tests written against any
140asynchronous operator should be written in [Marble Test Style outlined in detail here](doccs_app/content/guide/testing/internal-marble-tests.md).
141
142Each operator under test must be in its own file to cover the following cases:
143
144- Never
145- Empty
146- Single/Multiple Values
147- Error in the sequence
148- Never ending sequences
149- Early disposal in sequences
150
151If the operator accepts a function as an argument from the user/developer (for example `filter(fn)` or `zip(a, fn)`),
152then it must cover the following cases:
153
154- Success with all values in the callback
155- Success with the context, if any allowed in the operator signature
156- If an error is thrown
157
158### CI Tests
159
160- Using [Travis](https://travis-ci.org/) on your forked version of RxJS will allow running CI tests on that fork before submitting a PR to master
161- Simply create a `Travis` account and add your fork as a new project
162- [Sauce Labs](https://saucelabs.com/) setup will allow performing automated browser tests on the fork. Since `saucelabs` doesn't perform browser tests on a PR, this will help verify test results before PR's are checked into master.
163- In your `Travis` repo configuration, set the environment variables SAUCE_USERNAME and SAUCE_ACCESS_KEY to your `saucelabs` account ([reference](https://cloud.githubusercontent.com/assets/1210596/12679038/b9ba4eb6-c656-11e5-8c9b-b063c9a3f9dc.png))
164- As master runs both of these tests per each check in, it'd be welcome to setup those test before creating your PR
165
166## Performance Tests
167
168One of the primary goals of this library is (and will continue to be) great performance. As such, we've employed a variety of performance
169testing techniques.
170
171- DON'T labor over minute variations in ops/sec or milliseconds, there will always be variance in perf test results.
172- DON'T alter a performance test unless absolutely necessary. Performance tests may be compared to previous results from previous builds.
173- DO run tests multiple times and make sure the margins of error are low
174- DO run tests in your feature branches and compare them to master
175- DO add performance tests for all new operators
176- DO add performance tests that you feel are missing from other operators
177- DO add additional performance tests for all worthy code paths. If you develop an operator with special handling for scalar observables,
178 please add tests for those scenarios
179
180### Macro
181
182[Macro performance tests](perf/macro) are best written for scenarios where many object instance allocations (or deallocations) are occurring. Operators
183that create a lot of child subscriptions, or operators that emit new objects like Observables and Subjects are definitely worth creating
184macro performance tests for.
185
186Other scenarios for macro performance testing may include common end-to-end scenarios from real-world apps. If you have a situation in your
187app where you feel RxJS is performing poorly, please [submit an issue](https://github.com/ReactiveX/rxjs/issues/) and include a minimal code example showing
188your performance issues. We would love to solve perf for your real-world problems and add those tests to our perf test battery.
189
190Macro performance tests can be run by hosting the root directory with any web server (we use [http-server](https://www.npmjs.com/package/http-server)),
191then running:
192
193```sh
194npm run build_all
195protractor protractor.conf.js
196```
197
198### Micro
199
200[Micro performance tests](perf/micro) really only serve to test operations per second. They're quick and easy to develop, and provide a reasonable look into the
201relative performance of our operators versus prior versions. All operators should have corresponding micro performance tests.
202
203Micro performance test can be run with:
204
205```sh
206npm run build_all
207node perf/micro
208```
209
210If you wish to run a single micro performance test, you can do so by providing a single argument with the name of the perf test file(s):
211
212```sh
213node perf/micro zip
214```
215
216## Commit Message Guidelines
217
218We have very precise rules over how our git commit messages can be formatted. This leads to **more
219readable messages** that are easy to follow when looking through the **project history**. But also,
220we use the git commit messages to **generate the RxJS change log**. Helper script `npm run commit`
221provides command line based wizard to format commit message easily.
222
223### Commit Message Format
224
225Each commit message consists of a **header**, a **body** and a **footer**. The header has a special
226format that includes a **type**, a **scope** and a **subject**:
227
228```
229<type>(<scope>): <subject>
230<BLANK LINE>
231<body>
232<BLANK LINE>
233<footer>
234```
235
236The **header** is mandatory and the **scope** of the header is optional.
237
238Any line of the commit message cannot be longer than 100 characters! This allows the message to be easier
239to read on GitHub as well as in various git tools.
240
241### Revert
242
243If the commit reverts a previous commit, it should begin with `revert:`, followed by the header of the reverted commit. In the body it should say: `This reverts commit <hash>.`, where the hash is the SHA of the commit being reverted.
244
245### Type
246
247Must be one of the following:
248
249- **feat**: A new feature
250- **fix**: A bug fix
251- **docs**: Documentation only changes
252- **style**: Changes that do not affect the meaning of the code (white-space, formatting, missing
253 semi-colons, etc)
254- **refactor**: A code change that neither fixes a bug nor adds a feature
255- **perf**: A code change that improves performance
256- **test**: Adding missing tests
257- **chore**: Changes to the build process or auxiliary tools and libraries such as documentation
258 generation
259
260### Scope
261
262The scope could be anything specifying the place of the commit change. For example `Observable`, `Subject`, `switchMap`, etc.
263
264### Subject
265
266The subject contains succinct description of the change:
267
268- use the imperative, present tense: "change" not "changed" nor "changes"
269- don't capitalize first letter
270- no dot (.) at the end
271
272### Body
273
274Just as in the **subject**, use the imperative, present tense: "change" not "changed" nor "changes".
275The body should include the motivation for the change and contrast this with previous behavior.
276
277### Footer
278
279The footer should contain any information about **Breaking Changes** and is also the place to
280reference GitHub issues that this commit **Closes**.
281
282**Breaking Changes** should start with the word `BREAKING CHANGE:` with a space or two newlines. The rest of the commit message is then used for this.