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

davcri · 2017-10-22T10:32:23Z

This is only a proposal, please review it and discuss on what we should add/remove/improve !

Things that should be still addressed:

How we treat PRs that update a stub article appending new content to the stub template

Added guidelines as discussed here: #2231

davcri · 2017-10-26T08:53:03Z

bbfbb59 : updated the guide with suggestions from @erictleung (from this issue) and @Ethan-Arrowood (from the Gitter channel)

HKuz

Very nice @davcri 👌 thanks for getting all of this documented. I mostly had very minor, nitty-type comments.

HKuz · 2017-10-27T11:45:38Z

README.md

+
+#### Adding Labels
+- **content** is for pull requests that modify the content of articles on the guide (e.g.: new articles or updating articles)
+- **duplicate** is for pull requests that have the same content


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

HKuz · 2017-10-27T11:49:26Z

README.md

+
+
+### PR Review Ordering
+Older pull requests are reviewed firsts.  


"firsts." => "first"

HKuz · 2017-10-27T11:50:15Z

README.md

+Older pull requests are reviewed firsts.  
+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


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

HKuz · 2017-10-27T11:54:48Z

README.md

+![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).


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.

HKuz · 2017-10-27T11:57:31Z

README.md

+- 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)


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

HKuz · 2017-10-27T11:59:06Z

README.md

+- 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)
+
+Remember that a PR can always be reopened; merge can be reverted.


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 😄

HKuz · 2017-10-27T11:59:28Z

README.md

+
+Remember that a PR can always be reopened; merge can be reverted.
+
+#### Template text for closing PR


"closing PR" => "closing a PR"

HKuz · 2017-10-27T12:00:37Z

README.md

+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:


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

HKuz · 2017-10-27T12:05:14Z

README.md

+There will be probably merge conflicts; if it is the case, notify the contributor with this template text:
+
+#### 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.


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"

HKuz · 2017-10-27T12:06:40Z

README.md

+- 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)


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

davcri · 2017-10-27T14:37:23Z

I pushed a new commit. @HKuz thank you very much for the review. There were a lot of errors, I absolutely need to improve my English 😝

However: should we put this "reviewers' guide" to the README ? New contributors could be shocked by the wall of text.
What about putting it in a CONTRIBUTING.md file ?

HKuz · 2017-10-27T19:09:33Z

@davcri - no worries - I couldn't write that in another language! Yeah, there has been talk of creating a CONTRIBUTING.md file. I think these changes would be better suited there, and we keep the README as-is. If you want to create that file (root directory is fine), then copy the "How to contribute" and "How to run the Guide locally on your computer" from the README into it (keep those in the README as well) along with your changes here, we can work off that?

Add "How to contribute" section to CONTRIBUTING.md

davcri · 2017-10-28T08:24:30Z

I moved some text from the README to the CONTRIBUTING file.

@HKuz can you add here some relevant guideline from your comment ? You can commit directly on my branch.

HKuz · 2017-10-29T15:23:52Z

LGTM, @davcri - I added a couple sections (from that comment) and made some text edits on top of your PR. I'm going to merge this to get it in the repo, then any other changes/edits we can get in future PRs! ✅

davcri · 2017-10-29T18:39:10Z

Great! Thank you for your improvements 😄

Added "reviewers' guideline" to README.md

fa040c8

Added guidelines as discussed here: #2231

davcri mentioned this pull request Oct 22, 2017

Reviewers guide #2231

Closed

freeCodeCamp deleted a comment from tue96117 Oct 23, 2017

davcri requested review from HKuz and Bouncey October 24, 2017 08:29

Added guidelines for duplicates/conflicting PRs

bbfbb59

HKuz reviewed Oct 27, 2017

View reviewed changes

Fix typos in README

8235a68

Move "Reviewers'guide" to CONTRIBUTING.md.

2327d75

Add "How to contribute" section to CONTRIBUTING.md

Add comment edits to CONTRIBUTING.md

8e49b0a

HKuz approved these changes Oct 29, 2017

View reviewed changes

HKuz merged commit 5ecf1c0 into master Oct 29, 2017

raisedadead deleted the davcri-readme-patch branch November 1, 2017 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

davcri commented Oct 22, 2017 •

edited

Loading

davcri commented Oct 26, 2017

HKuz left a comment

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

HKuz Oct 27, 2017

davcri commented Oct 27, 2017

HKuz commented Oct 27, 2017

davcri commented Oct 28, 2017

HKuz commented Oct 29, 2017

davcri commented Oct 29, 2017



		### PR Review Ordering
		Older pull requests are reviewed firsts.


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

		#### Template text for closing PR

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

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

Conversation

davcri commented Oct 22, 2017 • edited Loading

davcri commented Oct 26, 2017

HKuz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davcri commented Oct 27, 2017

HKuz commented Oct 27, 2017

davcri commented Oct 28, 2017

HKuz commented Oct 29, 2017

davcri commented Oct 29, 2017

davcri commented Oct 22, 2017 •

edited

Loading