-
Notifications
You must be signed in to change notification settings - Fork 338
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #237 from ReactiveX/develop
v2
- Loading branch information
Showing
142 changed files
with
11,443 additions
and
3,845 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,39 @@ | ||
# Contributions Guidelines | ||
Contributions are always welcome. In fact, yours can shape the project this young. However, to make this a smooth collaboration experience for everyone and to maintain the quality of the code, here is a few things to consider before and after making a pull request: | ||
|
||
## Is it a feature or a patch? | ||
Before contributing, ask yourself if the change you're working on is a feature or a patch. A feature (like a new operator, function, etc.) needs a unit test with it, while a patch (like fixing a typo in `README.md`) may not. For small typos, it is always a good idea to just [email me](sendto:jo.chasinga@gmailcom) about it so we don't clutter the branch with super small changes. | ||
Contributions are always welcome. However, to make this a smooth collaboration experience for everyone and to maintain the quality of the code, here is a few things to consider before and after making a pull request: | ||
|
||
## Include a unit test | ||
Any change in the code other than small patches won't be accepted without a unit test. We use [testify/assert](https://github.com/stretchr/testify#assert-package) for the test. | ||
## Consistency | ||
|
||
## Read these great tips | ||
There are already +80 operators and +250 unit tests. Please don't try necessarily to reinvent the wheel and make sure to check first how the current implementation solves the most common problems. | ||
|
||
https://github.com/erlang/otp/wiki/Writing-good-commit-messages | ||
https://help.github.com/articles/closing-issues-via-commit-messages | ||
https://github.com/blog/1506-closing-issues-via-pull-requests | ||
## Edge Case | ||
|
||
There is no rigid rule or anything, just remember that a brief, succinct commit summary line can go a long way. In general, please don't include issue number the commit intends to resolve (see the above link for tip on how to do that), alphanumeric or any symbols, or past tense. The commit summary line should reads like a short directive like: | ||
When we develop a new operator, there are a lot of edge cases to handle (eager/lazy observation, sequential vs parallel, on error strategy, etc.). The utility functions `observable()`, `single()` and `optionalSingle()` are there to help. Yet, it is not always possible to use them (observation of multiple Observables, etc.). In this case, you may want to take a look at existing operators like `WindowWithTime` to see an exhaustive implementation. | ||
|
||
> Add feature A to class B | ||
## Unit Tests | ||
|
||
If you would like to be more elaborate, please see the first link on how to add that to the commit description. | ||
Make sure to include unit tests. Again, consistency is key. In most of the unit tests, we use the RxGo assertion API. | ||
|
||
## Open an issue | ||
This is to encourage discussions and reach a sound design decision before implementing an additional feature or fixing a bug. If you're proposing a new feature, make it obvious in the subject. | ||
|
||
## Create a feature branch | ||
Always create a branch dedicated to the feature or fix you intend to be merged into the base repository and keep it clean and commits minimal. | ||
## Duration | ||
|
||
## Squash your commits | ||
It is asked that you [squash your commits](https://github.com/blog/2141-squash-your-commits) down to a few, or one, discreet change sets before submitting a pull request. Fixing a bug will usually only need one commit, while a larger feature might contain a couple of separate improvements that is easier to track through different commits. | ||
If an operator input contains a duration, we should use `rxgo.Duration`. It allows us to mock it and to implement deterministic tests whenever possible using `timeCausality()`. | ||
|
||
## Include some comments | ||
It is not mandatory but rather nice to have a short comment accompanying a pull request. We're all humans here and sometime we lose tracks of things. | ||
## Write Nice Code | ||
|
||
## Write nice code | ||
Try to write idiomatic code according to [Go style guide](https://github.com/golang/go/wiki/CodeReviewComments). Also, see this project style guide for project-specific idioms (when in doubt, consult the first). | ||
|
||
## Code Formatting | ||
|
||
Before to create a pull request, make sure to format your code using: | ||
|
||
* [gofumpt](https://github.com/mvdan/gofumpt): | ||
* Install: `go get mvdan.cc/gofumpt` | ||
* Execute: `gofumpt -s -w .` | ||
|
||
* [goimports](https://godoc.org/golang.org/x/tools/cmd/goimports): | ||
* Install: `go get golang.o` | ||
* Execute: `goimports -w .` | ||
|
||
## Open an issue | ||
|
||
This is to encourage discussions and reach a sound design decision before implementing an additional feature or fixing a bug. If you're proposing a new feature, make it obvious in the subject. |
File renamed without changes.
Oops, something went wrong.