eslint/docs/src/maintainer-guide/pullrequests.md

   1 ---
   2 title: Reviewing Pull Requests
   3 layout: doc
   4 eleventyNavigation:
   5     key: reviewing pull requests
   6     parent: maintainer guide
   7     title: Reviewing Pull Requests
   8     order: 2
   9
  10 ---
  11
  12 Pull requests are submitted frequently and represent our best opportunity to interact with the community. As such, it's important that pull requests are well-reviewed before being merged and that interactions on pull requests are positive.
  13
  14 ## Who Can Review Pull Requests?
  15
  16 Anyone, both team members and the public, may leave comments on pull requests.
  17
  18 ## Reviewing a Pull Request
  19
  20 When a pull request is opened, the bot will check the following:
  21
  22 1. Has the submitter signed a CLA?
  23 1. Is the commit message summary in the correct format?
  24 1. Is the commit summary too long?
  25
  26 The bot will add a comment specifying the problems that it finds. You do not need to look at the pull request any further until those problems have been addressed (there's no need to comment on the pull request to ask the submitter to do what the bot asked - that's why we have the bot!).
  27
  28 Once the bot checks have been satisfied, you check the following:
  29
  30 1. Double-check that the commit message tag ("Fix:", "New:", etc.) is correct based on the issue (or, if no issue is referenced, based on the stated problem).
  31 1. If the pull request makes a change to core, ensure that an issue exists and the pull request references the issue in the commit message.
  32 1. Does the code follow our conventions (including header comments, JSDoc comments, etc.)? If not, please leave that feedback and reference the conventions document.
  33 1. For code changes:
  34     * Are there tests that verify the change? If not, please ask for them.
  35     * Is documentation needed for the change? If yes, please let the submitter know.
  36 1. Are there any automated testing errors? If yes, please ask the submitter to check on them.
  37 1. If you've reviewed the pull request and there are no outstanding issues, leave a comment "LGTM" to indicate your approval. If you would like someone else to verify the change, comment "LGTM but would like someone else to verify."
  38
  39 **Note:** If you are a team member and you've left a comment on the pull request, please follow up to verify that your comments have been addressed.
  40
  41 ## Who Can Merge a Pull Request
  42
  43 TSC members, Reviewers, Committers, and Website Team Members may merge pull requests, depending on the contents of the pull request.
  44
  45 Website Team Members may merge a pull request in the `eslint.org` repository if it is:
  46
  47 1. A documentation change
  48 1. A dependency upgrade
  49 1. A chore
  50
  51 Committers may merge a pull request if it is a non-breaking change and is:
  52
  53 1. A documentation change
  54 1. A bug fix (for either rules or core)
  55 1. A dependency upgrade
  56 1. Related to the build tool
  57 1. A chore
  58
  59 In addition, committers may merge any non-breaking pull request if it has been approved by at least one TSC member.
  60
  61 TSC members may merge all pull requests, including those that committers may merge.
  62
  63 ## When to Merge a Pull Request
  64
  65 We use the "Merge" button to merge requests into the repository. Before merging a pull request, verify that:
  66
  67 1. All comments have been addressed
  68 1. Any team members who made comments have verified that their concerns were addressed
  69 1. All automated tests are passing (never merge a pull request with failing tests)
  70
  71 Be sure to say thank you to the submitter before merging, especially if they put a lot of work into the pull request.
  72
  73 Team members may merge a pull request immediately if it:
  74
  75 1. Makes a small documentation change
  76 1. Is a chore
  77 1. Fixes a block of other work on the repository (build-related, test-related, dependency-related, etc.)
  78 1. Is an important fix to get into a patch release
  79
  80 Otherwise, team members should observe a waiting period before merging a pull request:
  81
  82 * Wait **2 days** if the pull request was opened Monday through Friday.
  83 * Wait **3 days** if the pull request was opened on Saturday or Sunday.
  84
  85 The waiting period ensures that other team members have a chance to review the pull request before it is merged.
  86
  87 If the pull request was created from a branch on the `eslint/eslint` repository (as opposed to a fork), delete the branch after merging the pull request. (GitHub will display a "Delete branch" button after the pull request is merged.)
  88
  89 **Note:** You should not merge your own pull request unless you're received feedback from at least one other team member.
  90
  91 ## When to Close a Pull Request
  92
  93 There are several times when it's appropriate to close a pull request without merging:
  94
  95 1. The pull request addresses an issue that is already fixed
  96 1. The pull request hasn't been updated in 30 days
  97 1. The pull request submitter isn't willing to follow project guidelines.
  98
  99 In any of these cases, please be sure to leave a comment stating why the pull request is being closed.
 100
 101 ### Example Closing Comments
 102
 103 If a pull request hasn't been updated in 30 days:
 104
 105 > Closing because there hasn't been activity for 30 days. If you're still interested in submitting this code, please feel free to resubmit.
 106
 107 If a pull request submitter isn't willing to follow project guidelines.
 108
 109 > Unfortunately, we can't accept pull requests that don't follow our guidelines. I'm going to close this pull request now, but if you'd like to resubmit following our guidelines, we'll be happy to review.