Skip to content

contributing guidelines review #457

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

Merged
merged 4 commits into from
Mar 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 58 additions & 60 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,30 @@

[`IPFS Main Repo`](https://github.com/ipfs/ipfs#ipfs---the-permanent-web) • [`IPFS Project Directory`](https://github.com/ipfs/ipfs#project-directory)

Want to hack on IPFS? Awesome! Here are instructions to get you started. They are not perfect yet. Please let us know what feels wrong or incomplete.

IPFS is an Open Source project and we welcome contributions of all sorts. There are many ways to help, from reporting issues, contributing code, and helping us improve our community.
Want to contribute to IPFS? Awesome! There are many ways to help, from reporting issues, contributing code, and helping us improve our community. Here are instructions to get you started. They are not perfect yet. Please let us know what feels wrong or incomplete.

## Topics

- [Security Issues](#security-issues)
- [Community Guidelines](#community-guidelines)
- [Dive Right In](#dive-right-in)
- [Reporting Issues](#reporting-issues)
- [Protocol Design](#protocol-design)
- [Implementation Design](#implementation-design)
- [Design Review](#design-review)
- [Community Improvement](#community-improvement)
- [Translations](#translations)
- [Helping in other ways](#helping-in-other-ways)
- [Code Contribution Guidelines](#code-contribution-guidelines)
- [**Security Issues**](#security-issues)
- [**Community Guidelines**](#community-guidelines)
- [**Looking for ways to contribute?**](#looking-for-ways-to-contribute)
- [Dive Right In](#dive-right-in)
- [Reporting Issues](#reporting-issues)
- [Community Improvement](#community-improvement)
- [Translations](#translations)
- [Helping in other ways](#helping-in-other-ways)
- [**Protocol Specification**](#protocol-specification)
- [Protocol Design](#protocol-design)
- [Implementation Design](#implementation-design)
- [Design Review](#design-review)
- [**Code Contribution Guidelines**](#code-contribution-guidelines)
- [Discuss big changes as Issues first](#discuss-big-changes-as-issues-first)
- [Pull Requests always welcome](#pull-requests-always-welcome)
- [Conventions](#conventions)
- [Go Code Contributing Guidelines](#go)
- [JavaScript Code Contributing Guidelines](#javascript)
- [JavaScript Code Contributing Guidelines](#javascript)
- [Git](#git)
- [Commit messages](#commit-messages)
- [Subject line should not be more than 80 characters long](#subject-line-should-not-be-more-than-80-characters-long)
- [A "License" and a "Signed-off-by" trailers are required](#a-license-and-a-signed-off-by-trailers-are-required)
- [Commit message examples](#commit-message-examples)
- [Code](#code)
- [Tests](#tests)
- [Documentation](#documentation)
Expand All @@ -55,34 +52,58 @@ If the issue is a protocol weakness or something not yet deployed, just discuss

## Community Guidelines

We want to keep the IPFS community awesome, growing and collaborative. We need your help to keep it that way. To help with this we've come up with some general guidelines for the community as a whole:

- Be nice: Be courteous, respectful and polite to fellow community members: no regional, racial, gender, or other abuse will be tolerated. We like nice people way better than mean ones!
- Encourage diversity and participation: Make everyone in our community feel welcome, regardless of their background and the extent of their contributions, and do everything possible to encourage participation in our community.
- Keep it legal: Basically, don't get anybody in trouble. Share only content that you own, do not share private or sensitive information, and don't break laws.
- Stay on topic: Make sure that you are posting to the correct channel and avoid off-topic discussions. Remember when you update an issue or respond to an email you are potentially sending to a large number of people. Please consider this before you update. Also remember that nobody likes spam.
We want to keep the IPFS community awesome, growing and collaborative. We need your help to keep it that way. Please review our [code-of-conduct](code-of-conduct.md).

There is also a more extensive [code-of-conduct](code-of-conduct.md).
## Looking for ways to contribute?

## Dive Right In
### Dive Right In

If you're curious to hack on IPFS right now and you just need an issue to focus on, check out [this search](https://github.com/search?utf8=%E2%9C%93&q=label%3A%22difficulty%3Aeasy%22+label%3A%22help+wanted%22+user%3AIPFS+is%3Aopen+&type=Issues) for issues tagged as "help wanted". Generally, these should be easier for newcomers, and are great places to start hacking away. Have fun!

## Reporting Issues
### Reporting Issues

If you find bugs, mistakes, inconsistencies in the IPFS project's code or documents, please let us know by filing an issue at the appropriate issue tracker (we use multiple repositories). No issue is too small.

The main issues for bug reporting are as follows:
- [go-ipfs/issues](https://github.com/ipfs/go-ipfs/issues) - Issues related to running the Go implementation of IPFS.
- [js-ipfs/issues](https://github.com/ipfs/js-ipfs/issues) - Issues related to running the js implementation of IPFS.
- [js-ipfs/issues](https://github.com/ipfs/js-ipfs/issues) - Issues related to running the JavaScript implementation of IPFS.
- [notes/issues](https://github.com/ipfs/notes/issues) - For general notes and ideas.
- [specs/issues](https://github.com/ipfs/specs/issues) - For protocol discussions.

The [go-ipfs](https://github.com/ipfs/go-ipfs) issues use a template that will guide you through the process of reporting a bug. We will be adding this kind of issue template to other repositories as bug reports become more common.

For all other questions/discussions, please visit the [discussion forum](https://discuss.ipfs.io)

## Protocol Design
### Community Tooling Improvement

The IPFS community requires maintenance of various "public infrastructure" resources. These include documentation, github repositories, CI build bots, and more. There is also helping new users with questions, spreading the word about IPFS, and so on. Soon, we will be planning and running conferences. Please get in touch if you would like to help out.

### Translations

Things that are ready to be translated can be found at https://www.transifex.com/ipfs/public/

This community moves very fast and documentation swiftly gets out of date. For
now, we are encouraging would-be translators to hold off from translating large
repositories, unless [they are listed on Transifex](https://www.transifex.com/ipfs/public/).

If you would like to add a translation for a project or a language that is not
on Transifex, please open an issue and ask the leads for a given repository to
get familiar with [ipfs/i18n](https://github.com/ipfs/i18n) before filing a PR,
so that there is no wasted effort.

If anyone has any issues understanding the English documentation, please let us know! We are very sensitive to language issues, and do not want to turn anyone away from hacking because of their language.

### IPFS Meetup

Running an IPFS Meetup is an excellent way to meet new friends that are also enthused by the Distributed Web. For tips and help, refer to [IPFS Meetups](https://github.com/ipfs/community#ipfs-meetups)

### Helping in other ways

Protocol Labs occasionally is able to hire developers for part time or full time positions, to work on IPFS. If you are interested, check out [the job listings](http://protocol.ai/join). If you'd like to help in other ways, propose your ideas on the IPFS forums at https://discuss.ipfs.io.

## Protocol Specification

### Protocol Design

When considering protocol design proposals, we are looking for:

Expand All @@ -94,48 +115,25 @@ When considering protocol design proposals, we are looking for:

Please note that protocol design is hard, and meticulous work. You may need to review existing literature and think through generalized use cases.

## Implementation Design
### Implementation Design

When considering design proposals for implementations, we are looking for:

- A description of the problem this design proposal solves
- Discussion of the tradeoffs involved
- Discussion of the proposed solution

## Design Review
### Design Review

Sometimes proposals can get stuck in either waiting for a decision or input. We've introduced "design reviews" as a forcing function to help stuck proposals make progress.

A good design review should start with:

* A clear proposal.
* A clear set of desired outputs and/or questions to be answered.
- A clear proposal.
- A clear set of desired outputs and/or questions to be answered.

To schedule a design review, show up to the [Core Implementations](https://github.com/ipfs/team-mgmt/issues/992) call and propose a design review in the design review section of the notes. When we get to the design reviews section of the call, you'll have a chance to make your case. If your design review is accepted, interested parties will sign-up for the review (directly in the meeting notes) and a DRI will be chosen to schedule the public design review meeting.

## Community Improvement

The IPFS community requires maintenance of various "public infrastructure" resources. These include documentation, github repositories, CI build bots, and more. There is also helping new users with questions, spreading the word about IPFS, and so on. Soon, we will be planning and running conferences. Please get in touch if you would like to help out.

## Translations

Things that are ready to be translated can be found at https://www.transifex.com/ipfs/public/

This community moves very fast and documentation swiftly gets out of date. For
now, we are encouraging would-be translators to hold off from translating large
repositories, unless [they are listed on Transifex](https://www.transifex.com/ipfs/public/).

If you would like to add a translation for a project or a language that is not
on Transifex, please open an issue and ask the leads for a given repository to
get familiar with [ipfs/i18n](https://github.com/ipfs/i18n) before filing a PR,
so that there is no wasted effort.

If anyone has any issues understanding the English documentation, please let us know! We are very sensitive to language issues, and do not want to turn anyone away from hacking because of their language.

## Helping in other ways

Protocol Labs occasionally is able to hire developers for part time or full time positions, to work on IPFS. If you are interested, check out [the job listings](http://protocol.ai/join). If you'd like to help in other ways, propose your ideas on the IPFS forums at https://discuss.ipfs.io.

## Code Contribution Guidelines

### Discuss big changes as Issues first
Expand Down Expand Up @@ -232,23 +230,23 @@ Update documentation when creating or modifying features. Test your documentatio

Pull requests descriptions should be as clear as possible. Err on the side of overly specific and include a reference to all related issues. If the pull request is meant to close an issue please use the Github keyword conventions of [closes, fixes, or resolves]( https://help.github.com/articles/closing-issues-via-commit-messages/). If the pull request only completes part of an issue use the [connects keywords]( https://github.com/waffleio/waffle.io/wiki/FAQs#prs-connect-keywords). This helps our tools properly link issues to pull requests.

### Code Review
#### Code Review

We take code quality seriously; we must make sure the code remains correct. We do code review on all changesets. Discuss any comments, then make modifications and push additional commits to your feature branch. Be sure to post a comment after pushing. The new commits will show up in the pull request automatically, but the reviewers will not be notified unless you comment.

We use the `needs review` as a signal that something needs review. If you can't add one to your own PR, feel free to request that the maintainers add the label to it.

### Rebasing
#### Rebasing

Pull requests **must be cleanly rebased ontop of master** without multiple branches mixed into the PR. If master advances while your PR is in review, please keep rebasing it. It makes all our work much less error-prone.

Before the pull request is merged, make sure that you squash your commits into logical units of work using `git rebase -i` and `git push -f`. After _every commit_ the test suite must be passing. This is so we can revert pieces, and so we can quickly bisect problems. Include documentation changes and tests in the same commit so that a revert would remove all traces of the feature or fix.

### Merge Approval
#### Merge Approval

We use LGTM (Looks Good To Me) in comments on the code review to indicate acceptance. A change **requires** LGTMs from the maintainers of each component affected. If you know whom it may be, ping them.

### Reverting Changes
#### Reverting Changes

When some change is introduced, and we decide that it isn't beneficial and/or it causes problems, we need to revert it.

Expand Down
96 changes: 15 additions & 81 deletions CONTRIBUTING_JS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# JavaScript Contributing Guildelines


These guidelines reflect our shared consensus on protocol and etiquette from what we've built so far. Every single item that is presented here is the result of lots of experimentation. However, that doesn't mean that there isn't a better way to do things. What we have below is simply what we've found to work best for us. In this document you will find notes about:

- Project structure.
Expand All @@ -25,16 +24,6 @@ Our toolkit for each of these is not set in stone, and we don't plan to halt our
- [Releasing](#releasing)
- [Documentation](#documentation)
- [Commits](#commits)
- [Commit Message Format](#commit-message-format)
- [Sign-off on commits](#sign-off-on-commits)
- [Revert](#revert)
- [Type](#type)
- [Scope](#scope)
- [Subject](#subject)
- [Body](#body)
- [Footer](#footer)
- [Examples](#examples)
- [References](#references)
- [Pull Requests](#pull-requests)
- [Aegir](#aegir)
- [...for maintainers](#for-maintainers)
Expand Down Expand Up @@ -109,7 +98,6 @@ NotFoundError.code = 'ERR_NOT_FOUND'
exports.NotFoundError = NotFoundError
```


This enables others to reuse those definitions and decreases the number of hardcoded values across our codebases.
For example:

Expand Down Expand Up @@ -156,72 +144,27 @@ For releasing a js-ipfs, see [RELEASE_ISSUE_TEMPLATE](https://github.com/ipfs/js

#### Documentation

We use [documentation.js](https://github.com/documentationjs/documentation/tree/master/docs) to document our JavaScript repositories. An example for how to use JSDoc to document everything can be seen in [this PR to js-ipfs](https://github.com/ipfs/js-ipfs/pull/651). Ideally, we create a `docs` folder in each repository, and make sure it is not tracked to git.

We use [`aegir-docs`](https://github.com/dignifiedquire/aegir) for the actual generation, which relies on JSDoc style comments. For more on aegir, see [the section below](#aegir).
`TBW`
Copy link
Member

Choose a reason for hiding this comment

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

Does this section want removing for the time being? What does TBW mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

To be written. Right now, AFAIK we don't use nor rely on documentation.js for anything.


### Commits

We have very precise rules over how our git commit messages can be formatted. This leads to more readable messages that are easy to follow when looking through the project history. But also, we use the git commit messages to generate the change log.
We have guidelines for how our git commit messages should be formatted. This leads to more readable messages that are easy to follow when looking through the project history. But also, we use the git commit messages to generate the change log.

The commit message formatting can be added using a typical git workflow or through the use of a CLI wizard ([Commitizen](https://github.com/commitizen/cz-cli)).

#### Commit Message Format
- **Type** - Must be one of the following:
- **feat**: A new feature
- **fix**: A bug fix
- **docs**: Documentation only changes
- **style**: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
- **refactor**: A code change that neither fixes a bug nor adds a feature
- **perf**: A code change that improves performance
- **test**: Adding missing tests
- **chore**: Changes to the build process or auxiliary tools and libraries such as documentation generation
- **Scope** - The scope could be anything specifying the place of the commit change. For example `api`, `cli`, etc...
- **Breaking Changes** - Should be identified at the end of commit message. Start with the words `BREAKING CHANGE:` on a new line followed by a space or two new lines. The rest of the commit message is then used to describe in detail what was broken and the migration path (if there is one).

Each commit message consists of a header, a body and a footer. The header has a special format that includes a type, a scope and a subject:

```
<type>(<scope>): <subject>
<BLANK LINE>
<body>
<BLANK LINE>
<footer>
```

The header is mandatory and the scope of the header is optional.

#### Revert

If 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.

#### Type

Must be one of the following:

* **feat**: A new feature
* **fix**: A bug fix
* **docs**: Documentation only changes
* **style**: Changes that do not affect the meaning of the code (white-space, formatting, missing
semi-colons, etc)
* **refactor**: A code change that neither fixes a bug nor adds a feature
* **perf**: A code change that improves performance
* **test**: Adding missing tests
* **chore**: Changes to the build process or auxiliary tools and libraries such as documentation
generation

#### Scope

The scope could be anything specifying the place of the commit change. For example `api`, `cli`, etc...

#### Subject

The subject contains a succinct description of the change:

* use the imperative, present tense: "change" not "changed" nor "changes"
* don't capitalize first letter
* no dot (.) at the end

#### Body

Just as in the subject, use the imperative, present tense: "change" not "changed" nor "changes". The body should include the motivation for the change and contrast this with previous behavior.

#### Footer

The footer should contain any information about breaking changes and is also the place to reference GitHub issues that this commit closes.

**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.

#### Examples
Examples:
Copy link
Member

Choose a reason for hiding this comment

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

Could you add an example with a breaking change message please?

Copy link
Member

Choose a reason for hiding this comment

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

@achingbrain there is already one below ;)

Copy link
Member

Choose a reason for hiding this comment

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

There's an example, but it's just a regular feature commit?

Copy link
Member Author

Choose a reason for hiding this comment

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

See

image


```
feat(pencil): add 'graphiteWidth' option
Expand All @@ -245,22 +188,13 @@ revert: feat(pencil): add 'graphiteWidth' option
This reverts commit 667ecc1654a317a13331b17617d973392f415f02.
```

#### References

This commit strategy is based on:

- https://github.com/conventional-changelog/conventional-changelog-angular/blob/master/convention.md
- https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit

More details about the commit convention can also be found in this [document](https://docs.google.com/document/d/1QrDFcIiPjSLDn3EL15IJygNPiHORgU1_OOAqWjiDU5Y).

### Pull Requests

There should be no dependencies that rely on commits. Instead, there should be WIP PR and each PR that depends on other WIP PR should list what it depends on. Yes, everyone will have to do the extra work of `npm link`ing everything, but this helps us have a cleaner workflow.

## Aegir

We've created [a module](https://github.com/dignifiedquire/aegir) to help us achieve all of the above with minimal effort. Feel free to also use it for your projects. Feedback is appreciated!
We've created [a module](https://github.com/ipfs/aegir) to help us achieve all of the above with minimal effort. Feel free to also use it for your projects. Feedback is appreciated!

#### ...for maintainers

Expand Down