Skip to content
This repository has been archived by the owner on Aug 28, 2019. It is now read-only.

README: proposal to add a "reviewers' guideline" #2976

Merged
merged 5 commits into from
Oct 29, 2017
Merged
Changes from 2 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
76 changes: 76 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ This repo is where we plan and maintain these Guide articles, which we then host
- [How to contribute](#how-to-contribute)
- [Running the Guide locally on your own computer](#running-the-guide-locally-on-your-own-computer)
- [Article style guide](#article-style-guide)
- [How we review pull requests](#how-we-review-pull-requests)
- [License](#license)


## What are Guide articles?
Guide articles can be an explanation of syntax, design pattern(s), what aria labels are for, or something like what the numbers mean in the top right-hand corner of your screen when at freecodecamp.org. You can find an [example article about HTML Elements here](./src/pages/html/elements/index.md).

Expand Down Expand Up @@ -208,6 +210,80 @@ Also, there's a community of support from a whole team of contributors, whom you

With your help, we can create a comprehensive reference tool that will help millions of people who are learning to code for years to come.

## How we review pull requests

### General guidelines
- we check that a pull request respects the [Article style guide](#article-style-guide)
- we follow general QA tips found in [Moderator guidelines](https://forum.freecodecamp.org/t/freecodecamp-moderator-guidelines/18295)
- as long as a pull request improves or expands the guide, we accept it even
if it contains imperfect English or partial content


### PR Review Ordering
Older pull requests are reviewed firsts.
Copy link
Member

Choose a reason for hiding this comment

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

"firsts." => "first"

You can use this filter to list relevant pull requests: [is:pr is:open sort:updated-asc -label:platform -label:enhancement -label:invalid -label:"changes requested"](https://github.com/freeCodeCamp/guides/pulls?utf8=%E2%9C%93&q=is%3Apr%20is%3Aopen%20sort%3Aupdated-asc%20-label%3Aplatform%20-label%3Aenhancement%20-label%3Ainvalid%20-label%3A%22changes%20requested%22)

### Accepting PR
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit: I think this reads better as "Accepting a PR". Similar comment below for "Closing PR" => "Closing a PR"


#### Squash commits
We use the __Squash and Merge__ option when merging the PR.
This will keep the commit history clean.

![GIF showing Squash functionality](https://files.gitter.im/FreeCodeCamp/Contributors/56MQ/9cb8db153d7bb1b3576cd1ffc207e39d.gif)

#### PR title
Currently under discussion, see [here](https://github.com/freeCodeCamp/guides/issues/1853).
Copy link
Member

Choose a reason for hiding this comment

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

I used your suggested format in the PULL_REQUEST_TEMPLATE (what contributors see when they are about to open a PR), with one minor change (verbs are in present/imperative tense instead of past tense - so "Python: add Variables article" vs. "Python: added Variables article"). I think we can update this section to include this and be consistent with the PR template.


- use meaningful titles
- avoid `Updating index.md`

In example: if you create a _Variables_ article inside the _Python_ directory,
the pull request title should be `Python: added "Variables" article`.

**Format**: `{Parent category}: added "{Article's title}" article`


### Closing PR

We close a pull request:
- if there is zero/little effort in it (e.g: copy pasting from another source like Wikipedia)
- if there is copied text from a copyrighted source (see also https://github.com/freeCodeCamp/guides/issues/2503)
- if it does not respect the [Article style guide](https://github.com/freeCodeCamp/guides#article-style-guide)
- if it does not respect the [Academic Honesty policy](https://www.freecodecamp.org/academic-honesty)
- if it is stale (if a change is requested and there weren't any activity for about 2 weeks)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "and there weren't" => "and there wasn't"


Remember that a PR can always be reopened; merge can be reverted.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this line in the README - it was more reassurance to new reviewers not to worry if they make a mistake 😄


#### Template text for closing PR
Copy link
Member

Choose a reason for hiding this comment

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

"closing PR" => "closing a PR"

> Thank you for your pull request. Please read this style guide: https://github.com/freeCodeCamp/guides#article-style-guide
I am closing this pull request for now. Please let me know if you have any questions.


### Conflicting and duplicate PRs
To find duplicates PRs:

1. Sort PR from the oldest
1. Search for PRs with similar content
1. Merge from the oldest to the newest

There will be probably merge conflicts; if it is the case, notify the contributor with this template text:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "will be probably" => "will probably be"


#### Template text
> We apologize for the inconvenience; however, while your PR was in the review queue someone else proposed the same file and their contribution was merged. As a result we need to resolve merge conflicts in order to merge your changes. If you're unsure about this process feel free to reach out in the contributor Gitter channel or comment below. We recommend you review the existing file and propose how you could incorporate your own ideas while maintaining the other contributors work. We will be closing this PR for now, but if you still want to see your changes added let us know and we can open it for additional commits.
Copy link
Member

Choose a reason for hiding this comment

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

Let's add an inline link to the contributor chat room: in the [Contributor Gitter channel](https://gitter.im/FreeCodeCamp/Contributors).... Also "while maintaining the other contributors work" => "while maintaining the other contributor's work"

Thank you for contributing! Keep up the awesome work

### Requesting changes

If a pull request is not perfect we can:
- request changes to the contributor and adding the *change requested* label (see below)
- fix minor issues by ourselves

#### Adding Labels
- **content** is for pull requests that modify the content of articles on the guide (e.g.: new articles or updating articles)
Copy link
Member

Choose a reason for hiding this comment

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

Another nit: "change requested" => "changes requested" (plural 😜 - same comment for the other two examples below)

- **duplicate** is for pull requests that have the same content
Copy link
Member

Choose a reason for hiding this comment

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

Add "as another open PR" to the end of this bullet

- **change requested** is for pull requests that need a change before getting merged.
- **stale**: is for pull requests with _"change requested"_ label that doesn't get activity after about 2 weeks. A _stale_ pull request should be closed (example: https://github.com/freeCodeCamp/guides/pull/235)

## License

Copyright (c) 2017 freeCodeCamp.
Expand Down