From 4afb407304c1ab11da2e380543eada522f4b5503 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Thu, 16 Jun 2016 15:14:42 -0700 Subject: [PATCH] Address comments from PR review. --- docs/avoiding-burnout.md | 2 +- docs/becoming-a-maintainer.md | 12 ++++++------ docs/merging-a-pull-request.md | 4 +--- docs/reviewing-a-pull-request.md | 4 ++++ docs/triaging-an-issue.md | 2 +- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/avoiding-burnout.md b/docs/avoiding-burnout.md index 4e8b2e0b..8ca27bfe 100644 --- a/docs/avoiding-burnout.md +++ b/docs/avoiding-burnout.md @@ -2,7 +2,7 @@ **This guide is for maintainers.** These special people have **write access** to one or more of Jekyll's repositories and help merge the contributions of others. You may find what is written here interesting, but it’s definitely not for everyone. -# 1. Use Homebrew +# 1. Use Jekyll Maintainers of Homebrew should be using it regularly. This is partly because you won't be a good maintainer unless you can put yourself in the shoes of our users but also because you may decide to stop using Jekyll and at that point you should also decide not to be a maintainer and find other things to work on. diff --git a/docs/becoming-a-maintainer.md b/docs/becoming-a-maintainer.md index facc9965..28f1f70b 100644 --- a/docs/becoming-a-maintainer.md +++ b/docs/becoming-a-maintainer.md @@ -8,23 +8,23 @@ So you want to become a maintainer of a Jekyll project? We'd love to have you! H You want to maintain Jekyll? Use it often. Do weird things with it. Do normal things with it. Does it work? Does it have any weaknesses? Is there a gap in the product that you think should be filled? -## 1. Help Triage Issues +## 2. Help Triage Issues Watch the repository you're interested in. Join [an Affinity Team](https://teams.jekyllrb.com) and receive mentions regarding a particular interest area of the project. When you receive a notification for an issue that has not been triaged by a maintainer, dive in. Can you reproduce the issue? Can you determine the fix? [More tips on Triaging an Issue in our maintainer guide](triaging-an-issue.md). Every maintainer loves an issue that is resolved before they get to it. :smiley: -## 2. Write Documentation +## 3. Write Documentation Good documentation means less confusion for our users and fewer issues to triage. Documentation is always in need of fixes and updates as we change the code. Read through the documentation during your normal usage of the product and submit changes as you feel they are necessary. -## 3. Write Code +## 4. Write Code -As a maintainer, you will be reviewing pull requests which update code. You should feel comfortable with the Jekyll codebase enough to confidently review a put forward. +As a maintainer, you will be reviewing pull requests which update code. You should feel comfortable with the Jekyll codebase enough to confidently review any pull request put forward. In order to become more comfortable, write some code of your own and send a pull request. A great place to start is with any issue labeled "bug" in the issue tracker. Write a test which replicates the problem and fails, then work on fixing the code such that the test passes. -## 4. Review Pull Requests +## 5. Review Pull Requests Start by reviewing one pull request a week. Leave detailed comments and [follow our guide for reviewing pull requests](reviewing-a-pull-request.md). -## 5. Ask! +## 6. Ask! Open an issue describing your contributions to the project and why you wish to be a maintainer. Issues are nice because you can easily reference where you have demonstrated that you help triage issues, write code & documentation, and review pull requests. You may also email any maintainer privately if you do not feel comfortable asking in the open. diff --git a/docs/merging-a-pull-request.md b/docs/merging-a-pull-request.md index ff1f102e..3d56b096 100644 --- a/docs/merging-a-pull-request.md +++ b/docs/merging-a-pull-request.md @@ -6,9 +6,7 @@ All pull requests should be subject to code review. Code review is a [foundational value](https://blog.fullstory.com/what-we-learned-from-google-code-reviews-arent-just-for-catching-bugs-b125a13aa292) of good engineering teams. Besides providing validation of correctness, it promotes a sense of community and gives other maintainers understanding of all parts of the code base. In short, code review is crucial to a healthy open source project. -Before merging a pull request **that changes code**, ensure that code is thoroughly reviewed and has received a thorough review from at least two maintainers. - -Before merging a pull request **that changes the documentation**, ensure there aren't any errors and that at least one other maintainer has agreed on the changes. +**Read our guide for [Reviewing a pull request](reviewing-a-pull-request.md) before merging.** Notably, the change must have tests if for code, and at least two maintainers must give it an OK. ## Merging diff --git a/docs/reviewing-a-pull-request.md b/docs/reviewing-a-pull-request.md index aeea06a5..feea47ec 100644 --- a/docs/reviewing-a-pull-request.md +++ b/docs/reviewing-a-pull-request.md @@ -24,6 +24,10 @@ Leave detailed comments wherever possible. Provide the contributor with context You may close a pull request if more than 30 days pass without a response from the author. +## Look for Tests + +If this is a code change, are there tests for the updated or added behaviour? Shipping a version with bugs is inevitable, but ensuring changes are tested helps keep bugs and regressions to a minimum. + ## CI Must Pass It is fine to ask a contributor to investigate failures on Travis and patch them up before you begin your review. It is helpful to leave a message for the contributor indicating that the tests have failed and that no review will occur before the tests pass. If they ask for help, take a look and assist if you can. diff --git a/docs/triaging-an-issue.md b/docs/triaging-an-issue.md index 75dbe4d1..d69b6457 100644 --- a/docs/triaging-an-issue.md +++ b/docs/triaging-an-issue.md @@ -6,4 +6,4 @@ Here are some key things to remember when evaluating an issue. ## Reproducible? -If the bug has clear reproduction steps, take a minute to try them. +If the bug has clear reproduction steps, take a minute to try them. If it helps, write a test in our test suite for the scenario which replicates the problem.