-
Notifications
You must be signed in to change notification settings - Fork 329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Twirp Design Principles to the Contributing Guidelines and archive protocol v6 #237
Changes from 1 commit
6248a28
f41bbe5
c432dd4
609a389
1f7d511
41e3719
0db5ec9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,38 +1,64 @@ | ||||||
# Contributing # | ||||||
# Contributing | ||||||
|
||||||
Thanks for helping make Twirp better! This is great! | ||||||
|
||||||
First, if you have run into a bug, please file an issue. We try to get back to | ||||||
issue reporters within a day or two. We might be able to help you right away. | ||||||
## Twirp Principles | ||||||
|
||||||
If you'd rather not publicly discuss the issue, please email [email protected] | ||||||
and/or [email protected]. | ||||||
Your contribution will go more smoothly if it aligns well with the project priorities. Twirp has been developed by twitch.tv and it has been in production for years. As many internal systems, teams and processes depend on Twirp, keeping backwards compatibility is taken very seriously. | ||||||
|
||||||
Issues are also a good place to present experience reports or requests for new | ||||||
features. | ||||||
Design principles: | ||||||
|
||||||
If you'd like to make changes to Twirp, read on: | ||||||
* Simplicity. Keep serialization and routing simple and intuitive. | ||||||
* Small api and interface. Reduce future support burden. | ||||||
* Prevent user errors. Avoid surprising behavior. | ||||||
* Pragmatism over being bleeding edge. | ||||||
* Hooks are for observability instead of control flow, so it is easier to debug Twirp systems. | ||||||
* No flags/options for code generation, so generated code is predictable across versions and platforms. | ||||||
* As few dependencies as possible, so it is easier to integrate and upgrade. | ||||||
* Prefer generated code over shared libraries between services and clients, so it is easier to implement changes without breaking compatibility. | ||||||
|
||||||
## Setup Requirements ## | ||||||
Contributions that are welcome: | ||||||
|
||||||
You will need git, Go 1.9+, and Python 2.7 installed and on your system's path. | ||||||
Install them however you feel. | ||||||
* Security updates. | ||||||
* Performance improvements. | ||||||
* Supporting new versions of Golang and Protobug. | ||||||
* Documentation. | ||||||
* Making Twirp easier to integrate with other useful tools. | ||||||
|
||||||
## Developer Loop ## | ||||||
In the other hand, contributions that contradict the following priorities will be more difficult: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
Generally you want to make changes and run `make`, which will install all | ||||||
dependencies we know about, build the core, and run all of the tests that we | ||||||
have against all of the languages we support. | ||||||
* Backwards compatibility is very important. Changes that break compatibility will see resistance, even when they only break a small use case. There must be a good reason behind a major version update. | ||||||
* Complex and involved features should be avoided. Twirp was designed to be easy to use and easy to maintain. See the streaming proposal as an example of a complex feature that was not able to make it into a major release: https://github.com/twitchtv/twirp/issues/3. | ||||||
* Features that could be implemented in a separate library should go in a separate library. Twirp allows easy integrtion with 3rd party code. | ||||||
|
||||||
|
||||||
## Report an Issue | ||||||
|
||||||
If you have run into a bug or want to discuss a new feature, please [file an issue](https://github.com/twitchtv/twirp/issues). If you'd rather not publicly discuss the issue, please email [email protected]. | ||||||
|
||||||
## Contributing Code with Pull Requests | ||||||
|
||||||
Twirp uses github pull requests. Fork, hack away at your changes and submit. Most pull requests will go through a few iterations before they get merged. Different contributors will sometimes have different opinions, and often patches will need to be revised before they can get merged. | ||||||
|
||||||
### Requirements | ||||||
|
||||||
Most tests of the Go server are in `internal/twirptest/service_test.go`. Tests | ||||||
of cross-language clients are in the [clientcompat](./clientcompat) directory. | ||||||
* Twirp officially supports the last 3 releases of Go. | ||||||
* The Python implementation uses Python 2.7. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Python has stopped official support for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is right. However this should be discussed in a different PR. The Python implementation has a few things that should be discussed all together. This PR adds some edits to the Contributing Guidelines, but with the focus on adding the Twirp Principles. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We edited this point with: "As such, it is not suitable for production use in its current form.". Thanks for the reference. |
||||||
* Protoc v3 to generate code. | ||||||
* For linters and other tools, we use [retool](https://github.com/twitchtv/retool). If `make setup` is not able to install it, you can install it in your path with `go get github.com/twitchtv/retool` and then install tools with `retool build`. | ||||||
|
||||||
## Contributing Code ## | ||||||
### Running tests | ||||||
|
||||||
Twirp uses github pull requests. Fork, hack away at your changes, run the test | ||||||
suite with `make`, and submit a PR. | ||||||
Generally you want to make changes and run `make`, which will install all | ||||||
dependencies we know about, build the core, and run all of the tests that we | ||||||
have against Go and Python code. A few notes: | ||||||
|
||||||
## Contributing Documentation ## | ||||||
* Make sure to clone the repo on `$GOPATH/src/github.com/twitchtv/twirp` | ||||||
* Run Go unit tests with `make test_core`, or just the tests with `go test -race ./...`. | ||||||
* Most tests of the Go server are in `internal/twirptest/service_test.go`. | ||||||
* Integration tests running the full stack in both Golang and Python auto-generated clients are in the [clientcompat](./clientcompat) directory. | ||||||
|
||||||
## Contributing Documentation | ||||||
|
||||||
Twirp's docs are generated with [Docusaurus](https://docusaurus.io/). You can | ||||||
safely edit anything inside the [docs](./docs) directory, adding new pages or | ||||||
|
@@ -46,24 +72,16 @@ Then, to render your changes, run docusaurus's local server. To do this: | |||||
3. `npm start` | ||||||
4. Navigate to http://localhost:3000/. | ||||||
|
||||||
## Releasing Versions ## | ||||||
## Making a New Release | ||||||
|
||||||
Releasing versions is the responsibility of the core maintainers. Most people | ||||||
don't need to know this stuff. | ||||||
|
||||||
Twirp uses [Semantic versioning](http://semver.org/): `v<major>.<minor>.<patch>`. | ||||||
|
||||||
* Increment major if you're making a backwards-incompatible change. | ||||||
* Increment minor if you're adding a feature that's backwards-compatible. | ||||||
* Increment patch if you're making a bugfix. | ||||||
|
||||||
To make a release, remember to update the version number in | ||||||
[internal/gen/version.go](./internal/gen/version.go). | ||||||
can skip this section. | ||||||
|
||||||
Twirp uses Github releases. To make a new release: | ||||||
|
||||||
1. Merge all changes that should be included in the release into the master | ||||||
branch. | ||||||
2. Update the version constant in `internal/gen/version.go`. | ||||||
2. Update the version constant in `internal/gen/version.go`. Make sure to respect [semantic versioning](http://semver.org/): `v<major>.<minor>.<patch>`. | ||||||
3. Add a new commit to master with a message like "Version vX.X.X release". | ||||||
4. Tag the commit you just made: `git tag <version number>` and `git push | ||||||
origin --tags` | ||||||
|
@@ -77,9 +95,9 @@ Twirp uses Github releases. To make a new release: | |||||
|
||||||
|
||||||
## Code of Conduct | ||||||
|
||||||
This project has adopted the [Amazon Open Source Code of Conduct](https://aws.github.io/code-of-conduct). | ||||||
For more information see the [Code of Conduct FAQ](https://aws.github.io/code-of-conduct-faq) or contact | ||||||
[email protected] with any additional questions or comments. | ||||||
For more information see the [Code of Conduct FAQ](https://aws.github.io/code-of-conduct-faq) or contact [email protected] with any additional questions or comments. | ||||||
|
||||||
|
||||||
## Licensing | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.