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

contributing guidelines review #457

Merged
merged 4 commits into from
Mar 3, 2020
Merged

contributing guidelines review #457

merged 4 commits into from
Mar 3, 2020

Conversation

daviddias
Copy link
Member

@daviddias daviddias commented Mar 3, 2020

  • Reorder sections on the main doc, clearly separate "ways to contribute" from "protocol specification" from "guidelines for PRs"
  • Remove duplication that existed in the Contributing_JS.md
  • Remove some stuff that we don't do

@daviddias daviddias requested a review from hsanjuan March 3, 2020 08:54
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM, couple of minor nits

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.

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

CONTRIBUTING_JS.md Outdated Show resolved Hide resolved
CONTRIBUTING_JS.md Outdated Show resolved Hide resolved
**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.

@achingbrain there is already one below ;)

daviddias and others added 2 commits March 3, 2020 15:20
Co-Authored-By: Alan Shaw <[email protected]>
Co-Authored-By: Alan Shaw <[email protected]>
@daviddias
Copy link
Member Author

Thank you for the review @alanshaw & @achingbrain. I took in your suggestions!

@daviddias daviddias merged commit 1022c4c into master Mar 3, 2020
@daviddias daviddias deleted the contributing-cleanup branch March 3, 2020 14:23
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.

3 participants