-
Notifications
You must be signed in to change notification settings - Fork 579
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
docs: readme revamp - contributing #1999
Closed
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
bda5406
docs: readme revamp - contributing
crodriguezvega a55f6fa
fixes
crodriguezvega 5da5fea
Merge branch 'main' into carlos/issue#1747-readme-revamp-contributing
crodriguezvega 702bc0a
refactor: contributing guidelines
crodriguezvega de16529
some typos
crodriguezvega e4b08ea
Merge branch 'main' into carlos/issue#1747-readme-revamp-contributing
crodriguezvega File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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 |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# Development setup | ||
|
||
## Forking | ||
|
||
Please note that Go requires code to live under absolute paths, which complicates forking. While my fork lives at `https://github.com/colin-axner/ibc-go`, the code should never exist at `$GOPATH/src/github.com/colin-axner/ibc-go`. Instead, we use `git remote` to add the fork as a new remote for the original repo `$GOPATH/src/github.com/cosmos/ibc-go`), and do all the work there. | ||
|
||
For instance, to create a fork and work on a branch of it, I would: | ||
|
||
- Create the fork on GitHub, using the fork button. | ||
- Go to the original repo checked out locally (i.e. `$GOPATH/src/github.com/cosmos/ibc-go`) | ||
- `git remote add fork git@github.com:colin-axner/ibc-go.git` | ||
|
||
Now `fork` refers to my fork and `origin` refers to the ibc-go version. So I can `git push -u fork main` to update my fork, and make pull requests to ibc-go from there. Of course, replace `colin-axner` with your git handle. | ||
|
||
To pull in updates from the origin repo, run | ||
|
||
- `git fetch origin` | ||
- `git rebase origin/main` (or whatever branch you want) | ||
|
||
## Dependencies | ||
|
||
We use [Go 1.14 Modules](https://github.com/golang/go/wiki/Modules) to manage dependency versions. | ||
|
||
The main branch of every Cosmos repository should just build with `go get`, which means they should be kept up-to-date with their dependencies, so we can get away with telling people they can just `go get` our software. | ||
|
||
Since some dependencies are not under our control, a third party may break our build, in which case we can fall back on `go mod tidy -v`. | ||
|
||
## Protobuf | ||
|
||
We use [Protocol Buffers](https://developers.google.com/protocol-buffers) along with [gogoproto](https://github.com/gogo/protobuf) to generate code for use in ibc-go. | ||
|
||
For determinstic behavior around protobuf tooling, everything is containerized using Docker. Make sure to have Docker installed on your machine, or head to [Docker's website](https://docs.docker.com/get-docker/) to install it. | ||
|
||
For formatting code in `.proto` files, you can run the `make proto-format` command. | ||
|
||
For linting and checking breaking changes, we use [buf](https://buf.build/). You can use the commands `make proto-lint` and `make proto-check-breaking` to respectively lint your proto files and check for breaking changes. | ||
|
||
To generate the protobuf stubs, you can run `make proto-gen`. | ||
|
||
We also added the `make proto-all` command to run all the above commands sequentially. | ||
|
||
For generating or updating the swagger file that documents the URLs of the RESTful API that exposes the gRPC endpoints over HTTP, you can run the `proto-swagger-gen` command. | ||
|
||
It reads protobuf service definitions and generates a reverse-proxy server which translates a RESTful HTTP API into gRPC. | ||
|
||
In order for imports to properly compile in your IDE, you may need to manually set your protobuf path in your IDE's workspace settings/config. | ||
|
||
For example, in vscode your `.vscode/settings.json` should look like: | ||
|
||
``` | ||
{ | ||
"protoc": { | ||
"options": [ | ||
"--proto_path=${workspaceRoot}/proto", | ||
"--proto_path=${workspaceRoot}/third_party/proto" | ||
] | ||
} | ||
} | ||
``` | ||
|
||
## Developing and testing | ||
|
||
- The latest state of development is on `main`. | ||
- Build the `simd` test chain binary with `make build`. | ||
- `main` must never fail `make test`. | ||
- No `--force` onto `main` (except when reverting a broken commit, which should seldom happen). | ||
- Create a development branch either on `github.com/cosmos/ibc-go`, or your fork (using `git remote add fork`). | ||
- Before submitting a pull request, begin `git rebase` on top of `main`. | ||
|
||
If you open a PR on ibc-go, it is mandatory to update the relevant documentation in `/docs`. | ||
|
||
Please make sure to run `make format` before every commit - the easiest way to do this is have your editor run it for you upon saving a file. Additionally please ensure that your code is lint compliant by running `make lint-fix` (requires `golangci-lint`). | ||
|
||
All Go tests in ibc-go can be ran by running `make test`. | ||
|
||
When testing a function under a variety of different inputs, we prefer to use [table driven tests](https://github.com/golang/go/wiki/TableDrivenTests). | ||
|
||
All tests should use the testing package. Please see the testing package [README](./testing/README.md) for more information. | ||
|
||
Test coverage is continuously deployed at https://app.codecov.io/github/cosmos/ibc-go. PRs that improve test coverage are welcome, but in general the test coverage should be used as a guidance for finding API use cases that are not covered by tests. We don't recommend adding tests that only improve coverage but not actually test a meaning use case. | ||
|
||
## Documentation | ||
|
||
- Generate the folder `docs/.vuepress/dist` with all the static files for the documentation site with `make build-docs`. | ||
- Run the documentation site locally with `make view-docs`. |
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 |
---|---|---|
@@ -0,0 +1,74 @@ | ||
|
||
# Go style guide | ||
|
||
In order to keep our code looking good with lots of programmers working on it, it helps to have a "style guide", so all the code generally looks quite similar. This doesn't mean there is only one "right way" to write code, or even that this standard is better than your style. But if we agree to a number of stylistic practices, it makes it much easier to read and modify new code. Please feel free to make suggestions if there's something you would like to add or modify. | ||
|
||
We expect all contributors to be familiar with [Effective Go](https://golang.org/doc/effective_go.html) (and it's recommended reading for all Go programmers anyways). Additionally, we generally agree with the suggestions in [Uber's style guide](https://github.com/uber-go/guide/blob/master/style.md) and use that as a starting point. | ||
|
||
## Code Structure | ||
|
||
Perhaps more key for code readability than good commenting is having the right structure. As a rule of thumb, try to write in a logical order of importance, taking a little time to think how to order and divide the code such that someone could scroll down and understand the functionality of it just as well as you do. A loose example of such order would be: | ||
|
||
- Constants, global and package-level variables | ||
- Main Struct | ||
- Options (only if they are seen as critical to the struct else they should be placed in another file) | ||
- Initialization / Start and stop of the service | ||
- Msgs/Events | ||
- Public Functions (In order of most important) | ||
- Private/helper functions | ||
- Auxiliary structs and function (can also be above private functions or in a separate file) | ||
|
||
## General | ||
|
||
* Use `gofumpt` to format all code upon saving it (or run `make format`). | ||
* Think about documentation, and try to leave godoc comments, when it will help new developers. | ||
* Every package should have a high level doc.go file to describe the purpose of that package, its main functions, and any other relevant information. | ||
* Applications (e.g. clis/servers) *should* panic on unexpected unrecoverable errors and print a stack trace. | ||
|
||
## Comments | ||
|
||
* Use a space after the comment deliminter (ex. `// your comment`). | ||
* Many comments are not sentences. These should begin with a lower case letter and end without a period. | ||
* Conversely, sentences in comments should be sentenced-cased and end with a period. | ||
|
||
## Linting | ||
|
||
* Run `make lint-fix` to fix any linting errors. | ||
|
||
## Various | ||
|
||
- Functions that return functions should have the suffix `Fn`. | ||
- Names should not [stutter](https://blog.golang.org/package-names). For example, a struct generally shouldn’t have a field named after itself; e.g., this shouldn't occur: | ||
|
||
``` golang | ||
type middleware struct { | ||
middleware Middleware | ||
} | ||
``` | ||
|
||
* In comments, use "iff" to mean, "if and only if". | ||
* Acronyms are all capitalized, like "RPC", "gRPC", "API". "MyID", rather than "MyId". | ||
* Prefer errors.New() instead of fmt.Errorf() unless you're actually using the format feature with arguments. | ||
|
||
## Importing libraries | ||
|
||
* Use [goimports](https://godoc.org/golang.org/x/tools/cmd/goimports). | ||
* Separate imports into blocks - one for the standard lib, one for external libs and one for application libs. | ||
|
||
## Dependencies | ||
|
||
* Dependencies should be pinned by a release tag, or specific commit, to avoid breaking `go get` when external dependencies are updated. | ||
* Refer to the [contributing](./development.md#dependencies) document for more details | ||
|
||
## Testing | ||
|
||
- Make use of table driven testing where possible and not-cumbersome. Read [this blog post](https://dave.cheney.net/2013/06/09/writing-table-driven-tests-in-go) for more information. | ||
- Make use of [assert](https://godoc.org/github.com/stretchr/testify/assert) and [require](https://godoc.org/github.com/stretchr/testify/require) | ||
- When using mocks, it is recommended to use Testify [mock](https://pkg.go.dev/github.com/stretchr/testify/mock) along with [Mockery](https://github.com/vektra/mockery) for autogeneration. | ||
|
||
## Errors | ||
|
||
- Ensure that errors are concise, clear and traceable. | ||
- Use stdlib errors package. | ||
- For wrapping errors, use `fmt.Errorf()` with `%w`. | ||
- Panic is appropriate when an internal invariant of a system is broken, while all other cases (in particular, incorrect or invalid usage) should return errors. |
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 |
---|---|---|
@@ -0,0 +1,46 @@ | ||
# Project structure | ||
|
||
If you're not familiar with the overall module structure from the SDK modules, please check this [document](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md) as prerequisite reading. | ||
|
||
Every Interchain Standard (ICS) has been developed in its own package. The development team separated the IBC TAO (Transport, Authentication, Ordering) ICS specifications from the IBC application level specification. The following sections describe the architecture of the most relevant directories that comprise this repository. | ||
|
||
## `modules` | ||
|
||
This folder contains implementations for the IBC TAO (`core`), IBC applications (`apps`) and light clients (`light-clients`). | ||
|
||
### `core` | ||
|
||
- `02-client`: This package is an implementation for Cosmos SDK-based chains of [ICS-02](https://github.com/cosmos/ibc/tree/main/spec/core/ics-002-client-semantics). This implementation defines the types and methods needed to operate light clients tracking other chain's consensus state. | ||
- `03-connection`: This package is an implementation for Cosmos SDK-based chains of [ICS-03](https://github.com/cosmos/ibc/tree/main/spec/core/ics-003-connection-semantics). This implementation defines the types and methods necessary to perform connection handshake between two chains. | ||
- `04-channel`: This package is an implementation for Cosmos SDK-based chains of [ICS-04](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics). This implementation defines the types and methods necessary to perform channel handshake between two chains and ensure correct packet sending flow. | ||
- `05-port`: This package is an implementation for Cosmos SDK-based chains of [ICS-05](https://github.com/cosmos/ibc/tree/main/spec/core/ics-005-port-allocation). This implements the port allocation system by which modules can bind to uniquely named ports. | ||
- `23-commitment`: This package is an implementation for Cosmos SDK-based chains of [ICS-23](https://github.com/cosmos/ibc/tree/main/spec/core/ics-023-vector-commitments). This implementation defines the functions required to prove inclusion or non-inclusion of particular values at particular paths in state. | ||
- `24-host`: This package is an implementation for Cosmos SDK-based chains of [ICS-24](https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements). | ||
|
||
### `apps` | ||
|
||
- `transfer`: This is the Cosmos SDK implementation of the [ICS-20](https://github.com/cosmos/ibc/tree/main/spec/app/ics-020-fungible-token-transfer) protocol, which enables cross-chain fungible token transfers. For more information, read the [module's docs](../apps/transfer/overview.md) | ||
- `27-interchain-accounts`: This is the Cosmos SDK implementation of the [ICS-27](https://github.com/cosmos/ibc/tree/main/spec/app/ics-027-interchain-accounts) protocol, which enables cross-chain account management built upon IBC. For more information, read the [module's documentation](../apps/interchain-accounts/overview.md). | ||
- `29-fee`: This is the Cosmos SDK implementation of the [ICS-29](https://github.com/cosmos/ibc/tree/main/spec/app/ics-029-fee-payment) middleware, which handles packet incentivisation and fee distribution on top of any ICS application protocol, enabling fee payment to relayer operators. For more information, read the [module's documentation](../middleware/ics29-fee/overview.md). | ||
|
||
### `light-clients` | ||
|
||
- `06-solomachine`: This package implement the types for the Solo Machine light client specified in [ICS-06](https://github.com/cosmos/ibc/tree/main/spec/client/ics-006-solo-machine-client). | ||
- `07-tendermint`: This package implement the types for the Tendermint consensus light client as specified in [ICS-07](https://github.com/cosmos/ibc/tree/main/spec/client/ics-007-tendermint-client). | ||
|
||
## `proto` | ||
|
||
This folder contains all the Protobuf files used for | ||
|
||
- common message type definitions, | ||
- message type definitions related to genesis state, | ||
- `Query` service and related message type definitions, | ||
- `Msg` service and related message type definitions. | ||
|
||
## `testing` | ||
|
||
This package contains the implementation of the testing package used in unit and integration tests. Please read the [package's documentation](../../testing/README.md) for more information. | ||
|
||
## `e2e` | ||
|
||
This folder contains all the e2e tests of ibc-go. Please read the [module's documentation](../../e2e/README.md) for more information. |
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 |
---|---|---|
@@ -0,0 +1,39 @@ | ||
# Pull request guidelines | ||
|
||
> To accommodate the review process we suggest that PRs are categorically broken up. Ideally each PR addresses only a single issue and does not introduce unrelated changes. Additionally, as much as possible code refactoring and cleanup should be submitted as separate PRs from bug fixes and feature additions. | ||
|
||
If the PR is the result of a related GitHub issue, please include `closes: #<issue number>` in the PR’s description in order to auto-close the related issue once the PR is merged. This will also link the ticket and the PR together so that if anyone looks at either in the future, they won’t have any issue trying to find the corresponding ticket/PR as it will be recorded in the sidebar. | ||
|
||
If the PR is not the result of an existing issue and it fixes a bug, please provide a detailed description of the bug. For feature addtions, we recommend opening an issue first and have it discussed and agreed upon, before working on it and opening a PR. | ||
|
||
Commit messages must follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/). This will help us to eventually move to automatic generation of changelogs. | ||
|
||
If possible, [tick the "Allow edits from maintainers" box](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork) when opening your PR from your fork of ibc-go. This allows us to directly make minor edits / refactors and speeds up the merging process. | ||
|
||
If you open a PR on ibc-go, it is mandatory to update the relevant documentation in `/docs`. | ||
|
||
## Pull request targeting | ||
|
||
Ensure that you base and target your PR on the `main` branch. Please make sure that the PR is made from a branch different than `main`. | ||
|
||
All development should be targeted against `main`. Bug fixes which are required for outstanding releases should be backported if the CODEOWNERS decide it is applicable. | ||
|
||
## Pull request review process | ||
|
||
All PRs require an approval from at least one CODEOWNER before merge. PRs which cause significant changes require two approvals from CODEOWNERS. When reviewing PRs please use the following review explanations: | ||
|
||
- `LGTM` without an explicit approval means that the changes look good, but you haven't pulled down the code, run tests locally and thoroughly reviewed it. | ||
- `Approval` through the GitHub UI means that you understand the code, documentation/spec is updated in the right places, you have pulled down and tested the code locally. In addition: | ||
- You must also think through anything which ought to be included but is not. | ||
- You must think through whether any added code could be partially combined (DRYed) with existing code. | ||
- You must think through any potential security issues or incentive-compatibility flaws introduced by the changes. | ||
- Naming must be consistent with conventions and the rest of the codebase | ||
- Code must live in a reasonable location, considering dependency structures (e.g. not importing testing modules in production code, or including example code modules in production code). | ||
- If you approve of the PR, you are responsible for fixing any of the issues mentioned here and more. | ||
- If you sat down with the PR submitter and did a pairing review please note that in the `Approval`, or your PR comments. | ||
- If you are only making "surface level" reviews, submit any notes as `Comments` without adding a review. | ||
|
||
## Pull request merge procedure | ||
|
||
- Ensure all GitHub requirements pass. | ||
- Squash and merge pull request. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I find this wording a little hard to read. Maybe
Or something like that?