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

Update reviewing and merging documentation #16915

Merged
merged 4 commits into from Sep 9, 2019

Conversation

@brentswisher
Copy link
Contributor

brentswisher commented Aug 5, 2019

The list of open pull requests for Gutenberg keeps growing. This is an attempt to update the developer documentation to encourage more people to participate in the reviewing and merging process, as discussed in the recent developer meeting - https://wordpress.slack.com/archives/C02QB2JS7/p1564580809002700

…ourage newer contributors
@brentswisher brentswisher requested a review from danielbachhuber Aug 5, 2019
@danielbachhuber danielbachhuber removed their request for review Aug 6, 2019
@@ -116,6 +116,8 @@ Every pull request goes through a manual code review, in addition to automated t

*As a contributor*, your responsibility is to learn from suggestions and iterate your pull request should it be needed based on feedback. Seek to collaborate and produce the best possible contribution to the greater whole.

Code reviews are encouraged by everyone who is willing to attempt one. If you review a pull request and are confident in the changes, approve it. If you don't feel totally confident it is ready for merging, add your review with a comment that says it should have another set of eyes on it before final approval. This can help filter out obvious bugs and simplify reviews for core members. It also is a great learning experience as you can follow the later reviews to see how to improve your own future reviews.

This comment has been minimized.

Copy link
@tellthemachines

tellthemachines Aug 9, 2019

Contributor

Might be good to add that it's also fine to just comment with questions when you're unsure of how something should work or if the reason for the change is unclear. Or review only the parts of the code you are comfortable with, and comment that changes x and y haven't been looked at yet.
Great to emphasise code review as a learning experience 🙂

This comment has been minimized.

Copy link
@brentswisher

brentswisher Sep 5, 2019

Author Contributor

Thanks for the feedback! I took a stab at what you were suggesting, let me know what you think.

This comment has been minimized.

Copy link
@tellthemachines

tellthemachines Sep 5, 2019

Contributor

Looks great, thanks for this!

@@ -138,7 +140,10 @@ A pull request can generally be merged once it is:

The final pull request merge decision is made by the **@wordpress/gutenberg-core** team.

Please make sure to assign your merged pull request to its release milestone. Doing so creates the historical legacy of what code landed when, and makes it possible for all project contributors (even non-technical ones) to access this information.
All members of the Wordpress organization on Github have the ability to review and merge pull requests. If you have reviewed a PR and are confident in the code, approve the pull request and comment pinging **@wordpress/gutenberg-core** or a specific core member who has been involved in the PR. Once they confirm there are no objections, you are free to merge the PR into master.

This comment has been minimized.

Copy link
@gziolo

gziolo Aug 27, 2019

Member

Typos:
Wordpress -> WordPress
Github -> GitHub

This comment has been minimized.

Copy link
@brentswisher

brentswisher Sep 5, 2019

Author Contributor

Thanks! Updated

@jasmussen

This comment has been minimized.

Copy link
Contributor

jasmussen commented Aug 27, 2019

With the changes suggested already, this seems good to me 👍 👍

mapk and others added 3 commits Sep 5, 2019
The sentence didn't sound quite right, so I hope you don't mind the update. If it's at all not what you wanted, feel free to revert it.
…ments if someone is not comfortable leaving a full review as suggested by @tellthemachines
@brentswisher brentswisher force-pushed the update/review-and-merging-documentation branch from ec4b6d0 to e7d7bc3 Sep 5, 2019
@mapk
mapk approved these changes Sep 6, 2019
Copy link
Contributor

mapk left a comment

:shipit:

@gziolo gziolo merged commit 57389db into master Sep 9, 2019
1 of 4 checks passed
1 of 4 checks passed
Filter opened Filter opened
Details
Filter opened Filter opened
Details
Milestone It Milestone It
Details
Travis CI - Pull Request Build Passed
Details
@gziolo gziolo deleted the update/review-and-merging-documentation branch Sep 9, 2019
@gziolo gziolo added this to the Gutenberg 6.5 milestone Sep 9, 2019
dd32 pushed a commit to dd32/gutenberg that referenced this pull request Sep 27, 2019
* Update reviewing and merging documentation to clarify process and encourage newer contributors

* Updated  a sentence for clarity

The sentence didn't sound quite right, so I hope you don't mind the update. If it's at all not what you wanted, feel free to revert it.

* Fix typos in updated documentation, including that dang capital "P"

* Add additional sentence to pull request documentation encouraging comments if someone is not comfortable leaving a full review as suggested by @tellthemachines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.