From 58d7db6e5d5344932857e008b3e9028b5aaac4e8 Mon Sep 17 00:00:00 2001 From: Tomas Sebestik Date: Wed, 8 Mar 2023 11:00:04 +0100 Subject: [PATCH] dangerjs: Improve check for git commit messages --- .gitlab/ci/pre_check.yml | 8 --- .gitlab/dangerjs/dangerfile.js | 2 - .gitlab/dangerjs/mrCommitsCommitMessage.js | 70 +++++++++++++++++---- .gitlab/dangerjs/mrCommitsTooManyCommits.js | 4 +- 4 files changed, 58 insertions(+), 26 deletions(-) diff --git a/.gitlab/ci/pre_check.yml b/.gitlab/ci/pre_check.yml index f99754d0c4..b7d35bf2d5 100644 --- a/.gitlab/ci/pre_check.yml +++ b/.gitlab/ci/pre_check.yml @@ -165,14 +165,6 @@ check_artifacts_expire_time: # check if we have set expire time for all artifacts - python tools/ci/check_artifacts_expire_time.py -check_commit_msg: - extends: .pre_check_template - script: - - git status - - git log -n10 --oneline ${PIPELINE_COMMIT_SHA} - # commit start with "WIP: " need to be squashed before merge - - 'git log --pretty=%s origin/master..${PIPELINE_COMMIT_SHA} -- | grep -i "^WIP:" && exit 1 || exit 0' - check_test_scripts_build_test_rules: extends: - .pre_check_template diff --git a/.gitlab/dangerjs/dangerfile.js b/.gitlab/dangerjs/dangerfile.js index b3b4a626c3..67fff7a80c 100644 --- a/.gitlab/dangerjs/dangerfile.js +++ b/.gitlab/dangerjs/dangerfile.js @@ -2,8 +2,6 @@ * Modules with checks are stored in ".gitlab/dangerjs/". To import them, use path relative to "dangerfile.js" */ -const checkMrCommits = require(`./mrCommitsCommitMessage.js`); - async function runChecks() { // Checks for merge request title require("./mrTitleNoDraftOrWip.js")(); diff --git a/.gitlab/dangerjs/mrCommitsCommitMessage.js b/.gitlab/dangerjs/mrCommitsCommitMessage.js index 787964e729..183970f9d5 100644 --- a/.gitlab/dangerjs/mrCommitsCommitMessage.js +++ b/.gitlab/dangerjs/mrCommitsCommitMessage.js @@ -1,26 +1,70 @@ /** - * Check commit message are descriptive enough (longer that 10 characters) + * Check if commit messages are sufficiently descriptive (not too short). * - * #TODO: this simple logic will be improved in future MRs - Jira IDF-6856. + * Search for commit messages that appear to be automatically generated by Gitlab or temporary messages and report them. * * @dangerjs WARN */ + module.exports = function () { - const shortCommitMessageThreshold = 10; // commit message is considered too short below this number of characters - const mrCommit = danger.gitlab.commits; + const mrCommits = danger.gitlab.commits; - let shortCommitMessages = []; - for (let i = 0; i < mrCommit.length; i++) { - const commitMessage = mrCommit[i].message; + const detectRegexes = [ + /^Rebased.*onto.*/i, // Automatically generated message by Gitlab + /^Fast-forwarded.*to.*/i, // Automatically generated message by Gitlab + /^Applied suggestion to.*/i, // Automatically generated message by Gitlab + /^Suggestion applied for line.*/i, // Automatically generated message by Gitlab + /^Opened merge request/i, // Automatically generated message by Gitlab + /^Merged.*/i, // Automatically generated message by Gitlab + /^Opened issue #\d+:.*/i, // Automatically generated message by Gitlab + /^Closed issue #\d+:.*/i, // Automatically generated message by Gitlab + /^Tagged.*as.*/i, // Automatically generated message by Gitlab + /^Deleted tag.*/i, // Automatically generated message by Gitlab + /^WIP.*/i, // Message starts with prefix "WIP" + /^Cleaned.*/i, // Message starts "Cleaned" + /clean ?up/i, // Message contains "clean up" + /^[^A-Za-z0-9\s].*/, // Message starts with special characters + ]; + // Search for the messages in each commit + let partMessages = []; + for (const commit of mrCommits) { + const commitMessage = commit.message; + const commitMessageTitle = commit.title; + + // Check if the commit message contains any Jira ticket references + const jiraTicketRegex = /[A-Z0-9]+-[0-9]+/g; + const jiraTicketMatches = commitMessage.match(jiraTicketRegex); + if (jiraTicketMatches) { + const jiraTicketNames = jiraTicketMatches.join(", "); + partMessages.push( + `- the commit message \`${commitMessageTitle}\` probably contains Jira ticket reference (\`${jiraTicketNames}\`). Please remove Jira tickets from commit messages.` + ); + continue; + } + + // Check if the commit message matches any regex from "detectRegexes" + if (detectRegexes.some((regex) => commitMessage.match(regex))) { + partMessages.push( + `- the commit message \`${commitMessageTitle}\` appears to be a temporary or automatically generated message` + ); + continue; + } + + // Check if the commit message is not too short + const shortCommitMessageThreshold = 20; // commit message is considered too short below this number of characters if (commitMessage.length < shortCommitMessageThreshold) { - shortCommitMessages.push(`- commit message: ${commitMessage}`); + partMessages.push( + `- the commit message \`${commitMessageTitle}\` may not be sufficiently descriptive` + ); } } - if (shortCommitMessages.length) { - warn(`Some of your commit messages may not be sufficiently descriptive (are shorter than ${shortCommitMessageThreshold} characters): - \n${shortCommitMessages.join("")} - \nYou might consider squashing commits (simplifying branch history) or updating those short commit messages.`); - } + // Create report + if (partMessages.length) { + partMessages.sort(); + let dangerMessage = `\nSome issues found for the commit messages in this MR:\n${partMessages.join('\n')} + \nPlease consider updating these commit messages. It is recommended to follow this [commit messages guide](https://gitlab.espressif.cn:6688/espressif/esp-idf/-/wikis/dev-proc/Commit-messages)`; + warn(dangerMessage); + } }; diff --git a/.gitlab/dangerjs/mrCommitsTooManyCommits.js b/.gitlab/dangerjs/mrCommitsTooManyCommits.js index 72625e1787..32a8cf96fa 100644 --- a/.gitlab/dangerjs/mrCommitsTooManyCommits.js +++ b/.gitlab/dangerjs/mrCommitsTooManyCommits.js @@ -1,12 +1,10 @@ /** * Check if MR has not an excessive numbers of commits (if squashed) * - * #TODO: this simple logic will be improved in future MRs - Jira IDF-6856. - * * @dangerjs INFO */ module.exports = function () { - const tooManyCommitThreshold = 5; // above this number of commits, squash is recommended + const tooManyCommitThreshold = 2; // above this number of commits, squash commits is suggested const mrCommits = danger.gitlab.commits; if (mrCommits.length > tooManyCommitThreshold) {