Skip to content

Commit

Permalink
feat: always treat PR title as the default commit message (#200)
Browse files Browse the repository at this point in the history
  • Loading branch information
mdjermanovic committed Sep 12, 2023
1 parent ae5c56c commit 7473faf
Show file tree
Hide file tree
Showing 9 changed files with 390 additions and 365 deletions.
20 changes: 3 additions & 17 deletions docs/commit-message-check.md
Expand Up @@ -4,24 +4,10 @@

`eslint-github-bot`'s commit message check ensures that all pull requests which get merged into `main` have a valid commit message, based on [ESLint's commit message guidelines](https://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes).

The ESLint team uses GitHub's "Squash and Merge" feature to merge pull requests. When using this feature, the default commit message for the squashed commit is determined as follows:

* If the pull request contains exactly one commit, that commit message is also used as the merge commit message.
* If the pull request contains more than one commit, the title of the pull request is used as the merge commit message.

To minimize the risk of an incorrect commit message getting merged into `main`, `eslint-github-bot` uses the same logic when evaluating a pull request; it checks the *commit message* if a pull request contains exactly one commit, and it checks the *title* if a pull request contains more than one commit.
The ESLint team uses GitHub's "Squash and Merge" feature to merge pull requests. When using this feature, the default commit message for the squashed commit is the title of the pull request. To minimize the risk of an incorrect commit message getting merged into `main`, `eslint-github-bot` checks the title.

## How do I fix it?

If this status check is failing on your pull request, you should first check what message the bot is referring to, by looking at the description next to the status check.

* If the description says "*Commit message* doesn't follow guidelines", you should amend your commit message from the command line to conform to [ESLint's commit message guidelines](https://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes). (This description will be used if your pull request has exactly one commit.)

```bash
git commit --amend
git push --force
```

* If the description says "*PR title* doesn't follow commit message guidelines", you should fix your pull request title on github.com to conform to [ESLint's commit message guidelines](https://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes). (This description will be used if your pull request has more than one commit.)
If this status check is failing on your pull request, you should fix your pull request title on github.com to conform to [ESLint's commit message guidelines](https://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes).

![](./edit-pr-title-explanation.png)
![](./edit-pr-title-explanation.png)
7 changes: 3 additions & 4 deletions src/plugins/commit-message/createMessage.js
Expand Up @@ -17,12 +17,11 @@ const ERROR_MESSAGES = {
/**
* Create a comment message body with the error details
* @param {Array<string>} errors list of the error codes
* @param {boolean} isTitle whether it is for title or the commit message
* @param {string} username username of the PR author
* @returns {string} the message to comment
* @private
*/
module.exports = function commentMessage(errors, isTitle, username) {
module.exports = function commentMessage(errors, username) {
const errorMessages = [];

errors.forEach(err => {
Expand All @@ -33,11 +32,11 @@ module.exports = function commentMessage(errors, isTitle, username) {

return `Hi @${username}!, thanks for the Pull Request
The **${isTitle ? "pull request title" : "first commit message"}** isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
The **pull request title** isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
${errorMessages.join("\n")}
**To Fix:** You can fix this problem by ${isTitle ? "clicking 'Edit' next to the pull request title at the top of this page." : "running `git commit --amend`, editing your commit message, and then running `git push -f` to update this pull request."}
**To Fix:** You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page.
Read more about contributing to ESLint [here](https://eslint.org/docs/developer-guide/contributing/)
`;
Expand Down
15 changes: 5 additions & 10 deletions src/plugins/commit-message/index.js
Expand Up @@ -10,7 +10,6 @@
// Requirements
//-----------------------------------------------------------------------------

const { getCommitMessageForPR } = require("../utils");
const commentMessage = require("./createMessage");
const { TAG_LABELS } = require("./util");

Expand Down Expand Up @@ -104,16 +103,14 @@ async function processCommitMessage(context) {
}

const allCommits = await github.pullRequests.listCommits(context.issue());
const messageToCheck = getCommitMessageForPR(allCommits.data, payload.pull_request);
const messageToCheck = payload.pull_request.title;
const errors = getCommitMessageErrors(messageToCheck);
let description;
let state;

if (errors.length === 0) {
state = "success";
description = allCommits.data.length === 1
? "Commit message follows guidelines"
: "PR title follows commit message guidelines";
description = "PR title follows commit message guidelines";

const labels = getCommitMessageLabels(messageToCheck);

Expand All @@ -123,12 +120,10 @@ async function processCommitMessage(context) {

} else {
state = "failure";
description = allCommits.data.length === 1
? "Commit message doesn't follow guidelines"
: "PR title doesn't follow commit message guidelines";
description = "PR title doesn't follow commit message guidelines";
}

// only check first commit message
// create status on the last commit
await github.repos.createStatus(
context.repo({
sha: allCommits.data[allCommits.data.length - 1].sha,
Expand All @@ -141,7 +136,7 @@ async function processCommitMessage(context) {

if (state === "failure") {
await github.issues.createComment(context.issue({
body: commentMessage(errors, allCommits.data.length !== 1, payload.pull_request.user.login)
body: commentMessage(errors, payload.pull_request.user.login)
}));
}

Expand Down
4 changes: 1 addition & 3 deletions src/plugins/release-monitor/index.js
Expand Up @@ -5,8 +5,6 @@

"use strict";

const { getCommitMessageForPR } = require("../utils");

const PATCH_COMMIT_MESSAGE_REGEX = /^(?:Build|Chore|Docs|Fix|Upgrade):/u;
const POST_RELEASE_LABEL = "patch release pending";
const RELEASE_LABEL = "release";
Expand Down Expand Up @@ -112,7 +110,7 @@ async function createAppropriateStatusForPR({ context, pr, pendingReleaseIssueUr
state: "success",
description: "No patch release is pending"
});
} else if (isMessageValidForPatchRelease(getCommitMessageForPR(allCommits, pr))) {
} else if (isMessageValidForPatchRelease(pr.title)) {
await createStatusOnPR({
context,
sha,
Expand Down
22 changes: 0 additions & 22 deletions src/plugins/utils.js

This file was deleted.

0 comments on commit 7473faf

Please sign in to comment.