Skip to content
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

Conversation

crodriguezvega
Copy link
Contributor

Description

  • Updates the contributing guidelines.
  • Adds contributing section to readme.

ref: #1747


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

Copy link
Member

@damiannolan damiannolan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job!

CONTRIBUTING.md Outdated
so further discussions can be made. We are following this process so all involved parties are in
agreement before any party begins coding the proposed implementation. Please use the [ADR template](./docs/architecture/adr-template.md)
to scaffold any new ADR. If you would like to see some examples of how these are written refer
to [Cosmos SDK ADRs](https://github.com/cosmos/cosmos-sdk/tree/master/docs/architecture)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using master would be a broken link?

Suggested change
to [Cosmos SDK ADRs](https://github.com/cosmos/cosmos-sdk/tree/master/docs/architecture)
to [Cosmos SDK ADRs](https://github.com/cosmos/cosmos-sdk/tree/main/docs/architecture)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering why the task wasn't failing, but the master link re-directs to main, we should still change it but I didn't realise it would do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reference our own ADR's now that we have some

Copy link
Contributor

@charleenfei charleenfei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @crodriguezvega for starting on this!

I think we should really consider making some significant changes to this document. Ideally everyone reads CONTRIBUTING.md. That means it should only contain information relevant to everyone. I think it should also be as minimal as possible and assume some prerequisite knowledge, so that contributors feel encouraged to read all the material (all the material being useful to be read). Maybe we can create a contributing directory within docs which contains more detailed information that might not be relevant for everyone, such as "beginner-guide.md" or "release-management.md" etc?

It'd be great to split this refactor up as well as there are a bit of diffs

README.md Outdated

To facilitate contributors the task of choosing what issues to pick up, we have the following two categories:
- Issues with the label [`good first issue`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) should be pretty well defined and are best suited for developers new to ibc-go.
- Issues with the label [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) are a bit more involved and they usually require some familiarity already with the codebase.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Issues with the label [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) are a bit more involved and they usually require some familiarity already with the codebase.
- Issues with the label [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) are a bit more involved and they usually require some familiarity with the codebase.

README.md Outdated
- Issues with the label [`good first issue`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) should be pretty well defined and are best suited for developers new to ibc-go.
- Issues with the label [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) are a bit more involved and they usually require some familiarity already with the codebase.

If you are interested to work on an issue, please make a comment on it; then we will be able to assign it to you. We will be happy to answer any questions you may have and help you out while you work on the issue.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you are interested to work on an issue, please make a comment on it; then we will be able to assign it to you. We will be happy to answer any questions you may have and help you out while you work on the issue.
If you are interested in working on an issue, please comment on it; then we will be able to assign it to you. We will be happy to answer any questions you may have and help you out while you work on the issue.

README.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated
Comment on lines 26 to 48
1. Either [open](https://github.com/cosmos/ibc-go/issues/new/choose) or
[find](https://github.com/cosmos/ibc-go/issues) an issue you'd like to help with.
Looking for a good place to start contributing?
- Check the out some [`good first issue`s](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22). These are issues whose scope of work should be pretty clearly specified and they are best suited for developers new to ibc-go (i.e. no deep knowledge of Cosmos SDK or ibc-go is required). For example, some of these issues may involve improving the logging, emitting new events or removing unsused code.
- Or pick up a [`help wanted`](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22) issue. These issues should be a bit more involved than the good first issues and the developer working on them would benefit from some familiarity with the codebase. This type of issues may involve adding new (or extending the functionality of existing) gRPC endpoints, bumping the version of Cosmos SDK or Tendermint or fixing bugs.
2. Participate in thoughtful discussion on that issue.
3. If you would like to contribute:
1. If the issue is a proposal, ensure that the proposal has been accepted
2. Ensure that nobody else has already begun working on this issue. If they have,
make sure to contact them to collaborate
3. If nobody has been assigned for the issue and you would like to work on it,
make a comment on the issue to inform the community of your intentions
to begin work
4. Follow standard Github best practices: fork the repo, branch from the
HEAD of `main`, make some commits, and submit a PR to `main`
1. If the issue is a proposal, ensure that the proposal has been accepted.
2. Ensure that nobody else has already begun working on this issue. If they have, make sure to contact them to collaborate.
3. If nobody has been assigned for the issue and you would like to work on it,
make a comment on the issue to inform the community of your intentions
to begin work. Then we will be able to assign the issue to you.
4. Follow standard GitHub best practices: fork the repo, branch from the
HEAD of `main`, make some commits, and submit a PR to `main`.
- For core developers working within the ibc-go repo, to ensure a clear
ownership of branches, branches must be named with the convention
`{moniker}/{issue#}-branch-name`
5. Be sure to submit the PR in `Draft` mode submit your PR early, even if
it's incomplete as this indicates to the community you're working on
something and allows them to provide comments early in the development process
6. When the code is complete it can be marked `Ready for Review`
7. Be sure to include a relevant change log entry in the `Unreleased` section
of `CHANGELOG.md` (see file for log format)
`{moniker}/{issue#}-branch-name`.
5. Feel free to submit the PR in `Draft` mode, even if
the work is not complete, as this indicates to the community you're working on
something and allows them to provide comments early in the development process.
6. When the code is complete it can be marked `Ready for Review`.
7. Be sure to include a relevant changelog entry in the `Unreleased` section
of [`CHANGELOG.md`](./CHANGELOG.md) (see file for log format).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to reference a document which explains how to contribute to open source projects for newcomers? I feel like this is too much unnecessary information for regular open source contributors. I'd like to have a section where seasoned contributors can come, read for a few minutes and understand how best to contribute to our codebase. This would involve limiting the amount of information they need to read. Thoughts?

CONTRIBUTING.md Outdated
Comment on lines 50 to 53
Note that for very small or blatantly obvious problems (such as typos) it is
not required to an open issue to submit a PR, but be aware that for more complex
problems/features, if a PR is opened before an adequate design discussion has
taken place in a github issue, that PR runs a high likelihood of being rejected.

Other notes:

- Looking for a good place to start contributing? How about checking out some
[good first issues](https://github.com/cosmos/ibc-go/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22)
- 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 `golangci-lint run`.
taken place in a GitHub issue, that PR runs a high likelihood of being rejected.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just state that every pr should have an accompanied issue. I'm sure folks opening up typo pr's won't read this document

CONTRIBUTING.md Outdated
Comment on lines 3 to 18
- [Contributing](#contributing)
- [Architecture Decision Records (ADR)](#architecture-decision-records-adr)
- [Dependencies](#dependencies)
- [Protobuf](#protobuf)
- [Forking](#forking)
- [Developing and testing](#developing-and-testing)
- [Pull Requests](#pull-requests)
- [PR Targeting](#pr-targeting)
- [Process for reviewing PRs](#process-for-reviewing-prs)
- [PR merge procedure](#pr-merge-procedure)
- [Updating Documentation](#updating-documentation)
- [Forking](#forking)
- [Dependencies](#dependencies)
- [Protobuf](#protobuf)
- [Testing](#testing)
- [Branching Model and Release](#branching-model-and-release)
- [PR Targeting](#pr-targeting)
- [Development Procedure](#development-procedure)
- [Pull Merge Procedure](#pull-merge-procedure)
- [Release Procedure](#release-procedure)
- [Point Release Procedure](#point-release-procedure)
- [New major release branch](#new-major-release-branch)
- [New minor release branch](#new-minor-release-branch)
- [Point release procedure](#point-release-procedure)
- [Post-release procedure](#post-release-procedure)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think needing to have a table of contents means we have too much information in this document, in my opinion. I think the shorter the document, the more likely folks will be to read all of it

CONTRIBUTING.md Outdated
so further discussions can be made. We are following this process so all involved parties are in
agreement before any party begins coding the proposed implementation. Please use the [ADR template](./docs/architecture/adr-template.md)
to scaffold any new ADR. If you would like to see some examples of how these are written refer
to [Cosmos SDK ADRs](https://github.com/cosmos/cosmos-sdk/tree/master/docs/architecture)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reference our own ADR's now that we have some

CONTRIBUTING.md Outdated
Comment on lines 148 to 152
## Pull Requests

## Branching Model and Release

User-facing repos should adhere to the trunk based development branching model: https://trunkbaseddevelopment.com/.

ibc-go utilizes [semantic versioning](https://semver.org/).
To accommodate review process we suggest that PRs are categorically broken up.
Ideally each PR addresses only a single issue. Additionally, as much as possible
code refactoring and cleanup should be submitted as separate PRs from bug fixes and feature additions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If contributors had to read one thing, I'd want it to be this

CONTRIBUTING.md Outdated
Comment on lines 186 to 207
### New major release branch

Pre-requisites for creating a release branch for a new major version:

1. Bump [Go package version](https://github.com/cosmos/ibc-go/blob/main/go.mod#L3).
2. Change all imports. For example: if the next major version is `v3`, then change all imports starting with `github.com/cosmos/ibc-go/v2` to `github.com/cosmos/ibc-go/v3`).

Once the above pre-requisites are satified:

1. Start on `main`.
2. Create the release branch (`release/vX.XX.X`). For example: `release/v3.0.x`.

### New minor release branch

1. Start on the latest release branch in the same major release line. For example: the latest release branch in the `v3` release line is `v3.2.x`.
2. Create branch from the release branch. For example: create branch `release/v3.3.x` from `v3.2.x`.

Post-requisites for both new major and minor release branches:

1. Add branch protection rules to new release branch.
2. Add backport task to [`mergify.yml`](https://github.com/cosmos/ibc-go/blob/main/.github/mergify.yml).
3. Create label for backport (e.g.`backport-to-v3.0.x`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think we should move this information to some other document. It is only relevant to the folks who do releases. I think CONTRIBUTING.md should only contain information we expect all developers to know

@@ -0,0 +1,74 @@

### Go style guide
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this from Tendermint and trimmed it down a bit. Feel free to comment on things that should and should not be here.

@crodriguezvega
Copy link
Contributor Author

I have broken up the original CONTRIBUTING.md file that we have and created a few separate files for different topics. I know this is a big PR with lots of changes, so if there's anything I can do to make it easier to review, please let me know.

Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to have a pr for each document here? Seems to me at least style guide and README can be separate prs?


This project adheres to ibc-go's [code of conduct](./CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code.

To facilitate contributors the task of choosing what issues to pick up, we have the following two categories:
Copy link
Contributor

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

To assist contributors in deciding which tasks to choose, we have added the following two issue labels:

Or something like that?

@crodriguezvega
Copy link
Contributor Author

Superseded by #2706 and related PRs.

@crodriguezvega crodriguezvega deleted the carlos/issue#1747-readme-revamp-contributing branch January 13, 2023 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants