Update maintainers documentation and contribution guides (#8939)

* Update contributing guide

In this change, I tried to simplify the language of the contributing
guide to make it more accessible and less wordy.

I also updated the information in a few places and fixed a couple of
typos.

* Remove table of contents

Since moving this doc to the docs site, our theme generates a ToC
automatically, hardcoding this one is redundant.

* Move the contributing guides

* Update maintainer documentation

This chnage updates the maintainers guide to include information on PR
reviews, deployments, and CI.

Housing this documentation here increases transparency with our
community.

* Edit with PR feedback

* Edit based on PR feedback
This commit is contained in:
Jacob Herrington 2020-06-30 02:11:59 -05:00 committed by GitHub
parent 3a69f349be
commit 8c4fd87599
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 292 additions and 244 deletions

View file

@ -19,4 +19,4 @@ We are all humans trying to work together to improve the community. Always be
kind and appreciate the need for trade-offs. ❤️
Before making your first pull request or issue, give our full
[Contributor's Guide](https://docs.dev.to/contributing_forem) a read.
[Contributor's Guide](https://docs.dev.to/contributing/forem) a read.

181
docs/contributing/forem.md Normal file
View file

@ -0,0 +1,181 @@
---
title: Contributing to Forem
---
# Contributing to Forem
We expect contributors to abide by our underlying
[Code of Conduct](https://dev.to/code-of-conduct). All discussions about this
project must be respectful and harassment-free.
Remember that communication is the lifeblood of any Open Source project. We are
all working on this together, and we are all benefiting from this software.
It's very easy to misunderstand one another in asynchronous, text-based
conversations. When in doubt, assume everyone has the best intentions.
If you feel anyone has violated our Code of Conduct, you should anonymously
contact the team with our [abuse report form](https://dev.to/report-abuse).
### Where to contribute
All [issues](https://github.com/thepracticaldev/dev.to/issues) labeled
[ready for dev](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aissue+is%3Aopen+label%3A%22ready+for+dev%22)
and
[type: bug](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aissue+is%3Aopen+label%3A%22type%3A+bug%22)
are up for grabs.
- [good first issue](https://github.com/thepracticaldev/dev.to/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22+)
issues are meant for newer developers.
- [difficulty: easy](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aopen+is%3Aissue+label%3A%22difficulty%3A+easy%22)
issues are usually confined to isolated areas of existing code.
- [difficulty: medium](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aopen+is%3Aissue+label%3A%22difficulty%3A+medium%22)
issues sometimes entail new features and might affect a significant area of
the codebase, but aren't overly complex.
- [difficulty: hard](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aopen+is%3Aissue+label%3A%22difficulty%3A+hard%22)
issues are typically far-reaching, and might need architecture decisions
during implementation. This label might also denote highly complex issues.
PRs without an associated issue may still be merged, but the core team will
focus on changes that solve existing issues. We strongly encourage creating an
issue before working on a PR!
When in doubt, ask a
[core team member](https://github.com/thepracticaldev/dev.to/#core-team) by
mentioning us on the issue.
**Documentation** is almost always a great place to start contributing to a new
project. Forem is an Open Source, community-driven project. Therefore, providing
and maintaining quality documentation is one of our most important jobs. You can
find more information in our
[docs guide](https://docs.dev.to/contributing/docs)!
**Refactoring**, which involves improving the code without modifying behavior,
is a great place to help out! Generally speaking, you can rely on existing tests
to ensure that your refactor doesn't introduce any unexpected behavior. If an
area isn't well tested, you might be asked to include a regression test with
your refactoring PR. Refactors can touch many files, so we encourage breaking
big changes into small PRs.
**Fixing bugs** is a super fast way to improve the experience for our users!
When you're fixing bugs, we appreciate communication in a GitHub issue. If an
issue exists, please claim that issue and link it in your PR, otherwise creating
an issue is the best first step! Be sure to surround bug fixes with ample tests;
bugs are magnets for other bugs. Write tests around bugs!
**Building features** requires a lot of communication, but we'd love to have
your help with this too! Features tend to be subjective and might spur some
debate. Be sure to create an issue for new features before getting started! If
your feature involves design changes, including design mockups can be very
helpful. As always, when in doubt, ask!
### How to contribute
1. [Fork the project](https://docs.dev.to/getting-started/forking/) and clone it
to your local machine. Follow the
[installation guide](https://docs.dev.to/installation/)!
2. Create a branch with your GitHub username and the ID of the
[issue](https://github.com/thepracticaldev/dev.to/issues), for example:
`git checkout -b USERNAME/some-new-feature-1234`
3. Code and commit your changes. Bonus points if you write a
[good commit message](https://chris.beams.io/posts/git-commit/):
`git commit -m 'Add some feature'`
4. Push to the branch: `git push -u origin USERNAME/some-new-feature-1234`
5. [Create a pull request](https://docs.dev.to/getting-started/pull-request/)
for your branch. 🎉
## Contribution guidelines
### Create an issue
Nobody's perfect. Something doesn't work? Something could be better? Check to
see if the issue already exists, and if it does, leave a comment to get our
attention! If the issue doesn't already exist, feel free to create a new one. A
core team member will triage incoming issues.
_Please note: core team members may update the title of an issue to reflect the
discussion._
### Please include tests
Some areas of the project could use updated tests, and new features should
always include test coverage. Please give our
[testing guide](https://docs.dev.to/tests/) a read!
### Code quality
We use [Code Climate](https://codeclimate.com/) to find code smells. If a pull
request contains code smells, we might recommend a refactor before merging. We
like readable code, and encourage DRY when it's reasonable!
More importantly, we avoid
[wrong abstractions](https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction).
Code quality tools are not perfect, so don't obsess over your Code Climate
score.
### Please use inclusive language
Inclusion and respect are core tenets of our
[Code of Conduct](https://dev.to/code-of-conduct). We expect thoughtful language
all the way down to the code. Some technical metaphors are alienating or
triggering. We ask that contributors go the extra mile to submit code which is
inclusive in nature.
If you unintentionally use language deemed harmful, there is no shame. We will
work together to find a better alternative. Being thoughtful about language also
encourages more thoughtful code!
### Create a pull request
- Try to keep the pull requests small. A pull request should try its very best
to address only a single concern.
- For work in progress pull requests, please use the
[Draft PR](https://github.blog/2019-02-14-introducing-draft-pull-requests/)
feature.
- Make sure all tests pass and add additional tests for the code you submit.
[More info here](https://docs.dev.to/tests/).
- Document your reasoning behind the changes. Explain why you wrote the code in
the way you did. The code should explain what it does.
- If there's an existing issue, reference to it by adding something like
`References/Closes/Fixes/Resolves #123`, where 123 is the issue number.
[More info here](https://github.com/blog/1506-closing-issues-via-pull-requests).
- Please fill out the PR Template when making a PR.
- All commits in a pull request will be squashed when merged.
_Please note: a core team member may close your PR if it has gone stale or if we
don't plan to merge the code._
### Pull request reviews
All community pull requests are reviewed by our core team.
- All contributors must sign the CLA.
- All required checks are expected to pass on each PR.
- In the case of flaky or unrelated test failures, a core team member will
restart CI.
- We require 2 approvals from core team members for each PR.
- Requested Changes must be resolved (with code or discussion) before merging.
- If you make changes to a PR, be sure to re-request a review.
- Style discussions are generally discouraged in PR reviews; make a PR to the
linter configurations instead.
- Your code will be deployed shortly after it is merged.
### A note on "force pushing"
After you submit your pull request, one of the members of the core team will
review your code.
Please avoid force pushing unless you need to rebase with the master branch.
If feedback is provided, any changes should be contained in new commits. Please
don't force push or worry about squashing your commits.
Force pushing (despite being useful) has some drawbacks. GitHub doesn't always
keep the review history, which results in lost context for the reviewers.
We squash every PR before merging, so there is no need to force push!
## The bottom line
We are all humans trying to work together to improve the community. Always be
kind and appreciate the need for tradeoffs. ❤️

View file

@ -0,0 +1,9 @@
---
title: Contributing Guide
items:
- docs.md
- forem.md
- api.md
---
# Contributing Guide

View file

@ -1,191 +0,0 @@
---
title: Contributing to Forem
---
# Contributing to Forem
- [Contributing](#contributing)
- [Where to contribute](#where-to-contribute)
- [How to contribute](#how-to-contribute)
- [Contribution guideline](#contribution-guidelines)
- [Create an issue](#create-an-issue)
- [Please include tests](#please-include-tests)
- [Code quality](#code-quality)
- [Create a pull request](#create-a-pull-request)
- [Pull requests reviews and "force pushing"](#pull-requests-reviews-and-force-pushing)
- [The bottom line](#the-bottom-line)
## Contributing
We expect contributors to abide by our underlying
[code of conduct](https://dev.to/code-of-conduct). All conversations and
discussions on GitHub (issues, pull requests) and across
[dev.to](https://dev.to) must be respectful and harassment-free.
Remember that communication is the lifeblood of any Open Source project. We are
all working on this together, and we are all benefiting from this software. It's
very easy to misunderstand one another over asynchronous, text-based
conversations: When in doubt, assume everyone you're interacting within this
project has the best intentions.
If you feel another member of the community has violated our Code of Conduct,
you may anonymously contact the team with our
[abuse report form](https://dev.to/report-abuse).
### Where to contribute
All [issues](https://github.com/thepracticaldev/dev.to/issues) labeled
[ready for dev](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aissue+is%3Aopen+label%3A%22ready+for+dev%22)
and
[type: bug](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aissue+is%3Aopen+label%3A%22type%3A+bug%22)
all are up for grabs.
- [good first issue](https://github.com/thepracticaldev/dev.to/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22+)
are issues meant for newer developers.
- [difficulty: easy](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aopen+is%3Aissue+label%3A%22difficulty%3A+easy%22)
issues are usually confined to isolated areas of existing code.
- [difficulty: medium](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aopen+is%3Aissue+label%3A%22difficulty%3A+medium%22)
issues can sometimes entail new features and might affect a significant area
of the codebase, but aren't overly complex.
- [difficulty: hard](https://github.com/thepracticaldev/dev.to/issues?q=is%3Aopen+is%3Aissue+label%3A%22difficulty%3A+hard%22)
issues will typically be far-reaching, and might require architecture
decisions during implementation. This label might also denote highly complex
issues.
While PRs without an associated `ready for dev` or `type: bug` labeled issue may
still be merged, please note that the core team will prioritize PRs that solve
existing issues. We strongly encourage creating an issue before working on a PR!
When in doubt, ask a
[core team member](https://github.com/thepracticaldev/dev.to/#core-team) by
mentioning us on the issue.
**Documentation** is almost always a great place to start contributing to a new
project. DEV is an Open Source, community-driven project, so providing and
maintaining quality documentation is one of our most important jobs. You can
find more information about contributing to the documentation in the
[docs/ directory](https://github.com/thepracticaldev/dev.to/blob/master/docs/contributing.md)!
**Refactoring** code, or improving the code without modifying the behavior, is
an area that can probably be done based on intuition and may not require much
communication to be merged. Generally speaking, you can rely on existing tests
to ensure that your refactoring doesn't introduce any unexpected behavior.
However, you might be asked to write a regression test if the area you've
refactored isn't well covered. As refactoring can span many files it's always
encouraged to proceed in steps if possible by submitting multiple smaller PRs.
**Fixing bugs** may also not require a lot of communication, but it's always
better to let us know what you're working on! Please surround bug fixes with
ample tests; bugs are magnets for other bugs. Write tests around bugs!
**Building features** is the area that will require the most communication
and/or negotiation. Every feature is subjective and open for debate. If your
feature involves user-facing design changes, please provide a mockup first so we
can all get on the same page. As always, when in doubt, ask!
### How to contribute
1. [Fork the project](https://docs.dev.to/getting-started/forking/) and clone it
to your local machine. Follow the initial setup
[here](https://docs.dev.to/installation/).
2. Create a branch with your GitHub username as a prefix and the ID of the
[issue](https://github.com/thepracticaldev/dev.to/issues) as a suffix, for
example: `git checkout -b USERNAME/that-new-feature-1234` or
`git checkout -b USERNAME/fixing-that-bug-1234` where `USERNAME` should be
replaced by your username and `1234` is the ID of the issue tied to your pull
request. If there is no issue, you can leave the number out.
3. Code and commit your changes. Bonus points if you write a
[good commit message](https://chris.beams.io/posts/git-commit/):
`git commit -m 'Add some feature'`.
4. Push to the branch: `git push -u origin USERNAME/that-new-feature-1234`.
5. [Create a pull request](https://docs.dev.to/getting-started/pull-request/)
for your branch. 🎉
## Contribution guidelines
### Create an issue
Nobody's perfect. Something doesn't work? Something could be done better? Check
to see if the issue already exists, and if it does, leave a comment to get our
attention! If the issue doesn't already exist, feel free to create a new one. A
core team member will triage incoming issues.
_Please note: core team members may update the title of an issue to more
accurately reflect the request/bug._
### Please include tests
Some existing code may be poorly written or untested, so we must have more
scrutiny going forward. We test with [RSpec](http://rspec.info/).
### Code quality
We use [CodeClimate](https://codeclimate.com/) to evaluate code smells. If a
pull request contributes a significant number of code smells, you may be asked
to refactor your change before it is merged. Focusing on writing reasonably DRY
code with a focus on readability will help avoid unnecessary code smells.
More importantly, we also try to avoid
[wrong abstractions](https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction).
Code quality tools are not perfect, so try not to obsess over your CodeClimate
score.
### Please use inclusive language
Inclusion and respect are core tenents of our
[code of conduct](https://dev.to/code-of-conduct) and we encourage thoughtful
language all the way down to the code. Some coding metaphors are alienating or
triggering and we ask that contributors go the extra mile to submit code which
is inclusive in nature.
If you unintentionally include language deemed harmful, there is no shame. We
will simply work together to settle on an alternative we are all comfortable
with. This extra care for naming tends to result in cleaner code as a byproduct.
### Create a pull request
- Try to keep the pull requests small. A pull request should try its very best
to address only a single concern.
- If you plan to do further work after the PR is submitted, please use the
[Draft PR](https://github.blog/2019-02-14-introducing-draft-pull-requests/)
feature.
- Make sure all tests pass and add additional tests for the code you submit.
[More info here](https://docs.dev.to/tests/).
- Document your reasoning behind the changes. Explain why you wrote the code in
the way you did. The code should explain what it does.
- If there's an existing issue related to the pull request, reference to it by
adding something like `References/Closes/Fixes/Resolves #305`, where 305 is
the issue number.
[More info here](https://github.com/blog/1506-closing-issues-via-pull-requests).
- Please fill out the PR Template when making a PR.
- All commits in a pull request will be squashed when merged, but when your PR
is approved and passes our CI, it will eventually be live on production!
_Please note: a core team member may close your PR if it has gone stale or if we
don't plan to merge the code._
### Pull requests reviews and "force pushing"
After you submit your pull request (PR), one of the members of the core team or
core contributors will likely do a review of the code accepting it or giving
feedback.
If feedback or suggestions are provided, any changes on your part should happen
in separate commits added to the existing ones.
Force pushing, though understandable for reasons of wanting to keep the history
clean has some drawbacks:
- it removes the review history of the code
- forces the reviewer to start from scratch when adding possible further
comments
PRs will be squashed and merged into master, so there's no need to use force
push.
Please avoid force pushing unless you need to rebase with the master branch.
## The bottom line
We are all humans trying to work together to improve the community. Always be
kind and appreciate the need for tradeoffs. ❤️

View file

@ -1,7 +1,25 @@
---
title: Deployment and CI/CD Process
title: Deployment Guide
---
# Deploying DEV
Anyone with the ability to merge PRs on GitHub can deploy the application.
Remember that this is a shared responsibility, so everyone should deploy code
occasionally.
If the application needs to be deployed when the code is merged, appending
`[deploy]` to the merge commit's message will trigger a deploy.
When deploying complex code, be sure that other team members are around to help
if something goes wrong.
Generally, it's a good idea to keep the SRE team in the loop on any deploys.
However, deployments are our collective responsibility, so it's important to
monitor your deploys. You can see deployment status on Travis-ci.com and in the
#deployment-pipeline channel on Slack. Be prepared to rollback or push a fix for
any deployment!
# Deployment and CI/CD Process
## Overview
@ -33,8 +51,10 @@ up between the different jobs. Here is a list of those additional checks that
are run.
- Job 0 is where we run Javascript tests, Preact tests, and coverage checks.
- Job 1 is where Travis builds Storybook to ensure its integrity, and where we check for any known vulnerabilities using `bundle-audit`.
- Job 2 is where Travis fires up a Rails console to ensure the application loads properly, and where specs with KnapsackPro are run.
- Job 1 is where Travis builds Storybook to ensure its integrity, and where we
check for any known vulnerabilities using `bundle-audit`.
- Job 2 is where Travis fires up a Rails console to ensure the application loads
properly.
If all of the jobs pass then we move on to Stage 2 of the Travis CI process.

View file

@ -0,0 +1,68 @@
---
title: Pull Requests
---
# Pull Requests
## General
- We generally follow a branch naming convention of
`<user>/<feature>-<issue number>`, e.g. `sloan/make-things-awesome-5555`.
Issue numbers can be omitted when unavailable, e.g. for things like quick
fixes, spikes, experiments, etc.
- Push early and often, you dont need to wait until youre finished to get
feedback. Please use GitHub draft PRs when you have a work-in-progress (WIP)
PR, or when you have a PR that is contingent on something else to be merged.
We prefer draft PRs rather than adding "DO NOT MERGE" to a PR's title. Any PR
that is not a draft should be ready to be merged by anyone once it has been
reviewed.
- Please follow our pull request template. Provide context for reviewers, when
in doubt err on the side of too much information.
- Core team members are not required to review draft PRs. To explicitly request
feedback, please assign or mention people.
## PR Reviews
PR review is the Core Team's opportunity to weigh-in on changes before they go
into production.
Keep in mind that our team is distributed across the world. It's a good idea to
leave far-reaching PRs open for a day, to give everyone a change to share their
thoughts.
- We require 2 approvals from core team members for each PR:
- One from the same team because they have context and are working towards the
same goal as you.
- One from outside your team for the following reasons:
- A different perspective from someone who has less context
- To leverage our different strengths and garner additional insights
spreading knowledge and code exposure throughout the team
- To avoid overloading individuals
- Tag people that youd like to review your PR using GitHubs Reviewers
function.
- Be kind! The goal here is to work together on shipping good code, not to judge
people.
- For serious issues, use Githubs “Request changes” feature. This should be
reserved for serious problems, e.g.
- Doesnt do what the issue said
- Foreseeable performance problems
- Provable security issues
- Breaks existing functionality
- Everything else can be posted as a comment, but its up to the original PR
author whether or not they want to incorporate the changes.
- Please keep style discussions to an absolute minimum, that time is better
spent making a PR for the configuration of our various lint tools.
- If you make changes to your PR, please re-request feedback from previous
reviewers.
# Merging Pull Requests
At the time of writing, we tend to prefer squashing PRs into a single commit and
merging them. This is easily (and safely) achieved using the GitHub UI.
All required checks such as CI and Code Climate should be green.
Once a PR is merged, it might need to be deployed. Deployment is a team
responsibility, and everyone on the core team should be comfortable deploying
code. For more information, read the
[deployment guide](https://docs.dev.to/maintainers/deployment).

View file

@ -1,48 +1,12 @@
---
title: Maintainers Guide
items:
- pull_requests.md
- deploying.md
---
# Maintainer Processes
This document contains basic instructions for the maintainers of the DEV
application. It's a work in-progress, but should allow the DEV Core Team to
document processes and strategies in a transparent way!
# Skipping CI
Some small PRs might seem like good candidates for skipping the CI suite. For
example, a change that only touches documentation copy or a tiny CSS tweak.
For the time being, we should avoid skipping CI in PRs because it causes some
confusion during the review process. However, it does make sense to skip CI on
merge commits that include extremely minor changes.
You can skip CI by appending [ci skip] to merge commit's message.
When in doubt, don't skip CI.
# Pull Request Review
At the moment, our PR review process is being evaluated, so this is subject to
change.
Before a PR is merged we expect:
- every contributor to sign our CLA
- all of the automated checks to pass
- approval from at least two members of the Core Team
PR review is the Core Team's opportunity to weigh-in on changes before they go
into master.
Keep in mind that our team is distributed across the world. It's a good idea to
leave far-reaching PRs open for a day, to give everyone a change to share their
thoughts.
# Merging Pull Requests
At the time of writing, we tend to prefer squashing PRs into a single commit and
merging them. This is easily (and safely) achieved using the GitHub UI.
# Deploying DEV
If the application needs to be deployed when the code is merged, appending
[deploy] to the merge commit's message will trigger a deploy.
Generally, it's a good idea to keep the SRE team in the loop on any deploys.

View file

@ -3,15 +3,12 @@ items:
- technical-overview
- installation
- getting-started
- contributing
- backend
- frontend
- internal
- design
- deployment.md
- tests
- contributing.md
- contributing_api.md
- contributing_forem.md
- faqs.md
- troubleshooting.md
- licensing.md
@ -68,7 +65,7 @@ for those changes to take effect.
# Contributing to the docs
If you're looking for more information on contributing, check out the
[Contributing article][contributing].
[Contributing guide][contributing].
[installation]: /installation/
[architecture]: /technical-overview/architecture/

View file

@ -32,4 +32,4 @@ helping to ensure your patch fixes the bug.
[acceptance_tests]: /tests/acceptance-tests/
[integration_tests]: /tests/integration-tests/
[ci]: /deployment/
[ci]: /maintainers/deploying/