Bläddra i källkod

Merge branch 'dangerjs/conventional_commits' into 'master'

Add DangerJS check for conventional commits

Closes IDF-7035

See merge request espressif/esp-idf!24000
Tomas Sebestik 2 år sedan
förälder
incheckning
09bc513b2c

+ 116 - 38
.gitlab/dangerjs/mrCommitsCommitMessage.js

@@ -1,72 +1,150 @@
 /**
- * Check if commit messages are sufficiently descriptive (not too short).
- *
- * Search for commit messages that appear to be automatically generated by Gitlab or temporary messages and report them.
+ * Check that commit messages are based on the Espressif ESP-IDF project's internal rules for git commit messages.
  *
  * @dangerjs WARN
  */
 
 module.exports = async function () {
     const mrCommits = danger.gitlab.commits;
+    const lint = require("@commitlint/lint").default;
 
-    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
+    const allowedTypes = [
+        "change",
+        "ci",
+        "docs",
+        "feat",
+        "fix",
+        "refactor",
+        "remove",
+        "revert",
     ];
+    const lintingRules = {
+        // rule definition: [(0-1 = off/on), (always/never = must be/mustn't be), (value)]
+        "body-max-line-length": [1, "always", 100], // Max length of the body line
+        "footer-leading-blank": [1, "always"], // Always have a blank line before the footer section
+        "footer-max-line-length": [1, "always", 100], // Max length of the footer line
+        "subject-max-length": [1, "always", 50], // Max length of the "Summary"
+        "subject-min-length": [1, "always", 20], // Min length of the "Summary"
+        "scope-case": [1, "always", "lower-case"], // "scope/component" must start with lower-case
+        // "scope-empty": [1, "never"], // "scope/component" is mandatory
+        "subject-case": [1, "always", ["sentence-case"]], // "Summary" must start with upper-case
+        "subject-full-stop": [1, "never", "."], // "Summary" must not end with a full stop (period)
+        "subject-empty": [1, "never"], // "Summary" is mandatory
+        "type-case": [1, "always", "lower-case"], // "type/action" must start with lower-case
+        "type-empty": [1, "never"], // "type/action" is mandatory
+        "type-enum": [1, "always", allowedTypes], // "type/action" must be one of the allowed types
+        "body-leading-blank": [1, "always"], // Always have a blank line before the body section
+    };
 
     // Search for the messages in each commit
-    let partMessages = [];
+    let issuesAllCommitMessages = [];
+
     for (const commit of mrCommits) {
         const commitMessage = commit.message;
         const commitMessageTitle = commit.title;
 
+        let issuesSingleCommitMessage = [];
+        let reportSingleCommitMessage = "";
+
         // 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.`
+            issuesSingleCommitMessage.push(
+                `- 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;
+        // Lint commit messages with @commitlint (Conventional Commits style)
+        const result = await lint(commit.message, lintingRules);
+
+        for (const warning of result.warnings) {
+            // Custom messages for each rule with terminology used by Espressif conventional commits guide
+            switch (warning.name) {
+                case "subject-max-length":
+                    issuesSingleCommitMessage.push(
+                        `- *summary* appears to be too long`
+                    );
+                    break;
+                case "type-empty":
+                    issuesSingleCommitMessage.push(
+                        `- *type/action* looks empty`
+                    );
+                    break;
+                case "type-case":
+                    issuesSingleCommitMessage.push(
+                        `- *type/action* should start with a lowercase letter`
+                    );
+
+                    break;
+                case "scope-empty":
+                    issuesSingleCommitMessage.push(
+                        `- *scope/component* looks empty`
+                    );
+                    break;
+                case "scope-case":
+                    issuesSingleCommitMessage.push(
+                        `- *scope/component* should start with a lowercase letter`
+                    );
+                    break;
+                case "subject-empty":
+                    issuesSingleCommitMessage.push(`- *summary* looks empty`);
+                    break;
+                case "subject-min-length":
+                    issuesSingleCommitMessage.push(
+                        `- *summary* looks too short`
+                    );
+                    break;
+                case "subject-case":
+                    issuesSingleCommitMessage.push(
+                        `- *summary* should start with a capital letter`
+                    );
+                    break;
+                case "subject-full-stop":
+                    issuesSingleCommitMessage.push(
+                        `- *summary* should not end with a period (full stop)`
+                    );
+                    break;
+                case "type-enum":
+                    issuesSingleCommitMessage.push(
+                        `- *type/action* should be one of [${allowedTypes
+                            .map((type) => `\`${type}\``)
+                            .join(", ")}]`
+                    );
+                    break;
+
+                default:
+                    issuesSingleCommitMessage.push(`- ${warning.message}`);
+            }
         }
 
-        // 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) {
-            partMessages.push(
-                `- the commit message \`${commitMessageTitle}\` may not be sufficiently descriptive`
-            );
+        if (issuesSingleCommitMessage.length) {
+            reportSingleCommitMessage = `- the commit message \`"${commitMessageTitle}"\`:\n${issuesSingleCommitMessage
+                .map((message) => `  ${message}`) // Indent each issue by 2 spaces
+                .join("\n")}`;
+            issuesAllCommitMessages.push(reportSingleCommitMessage);
         }
     }
 
     // Create report
-    if (partMessages.length) {
-        partMessages.sort();
-        let dangerMessage = `\nSome issues found for the commit messages in this MR:\n${partMessages.join(
+    if (issuesAllCommitMessages.length) {
+        issuesAllCommitMessages.sort();
+        const basicTips = [
+            `- correct format of commit message should be: \`<type/action>(<scope/component>): <Summary>\`, for example \`fix(esp32): Fixed startup timeout issue\``,
+            `- sufficiently descriptive message summary should be between 20 to 50 characters and start with upper case letter`,
+            `- avoid Jira references in commit messages (unavailable/irrelevant for our customers)`,
+            `- follow this [commit messages guide](${process.env.DANGER_GITLAB_HOST}/espressif/esp-idf/-/wikis/dev-proc/Commit-messages)`,
+        ];
+        let dangerMessage = `\n**Some issues found for the commit messages in this MR:**\n${issuesAllCommitMessages.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)`;
+			\n***
+			\n**Please consider updating these commit messages** - here are some basic tips:\n${basicTips.join(
+                "\n"
+            )}
+            \n***
+            `;
 
         // Create AI generated suggestion for git commit message based of gitDiff and current commit messages
         const AImessageSuggestion =

Filskillnaden har hållts tillbaka eftersom den är för stor
+ 748 - 12
.gitlab/dangerjs/package-lock.json


+ 2 - 1
.gitlab/dangerjs/package.json

@@ -6,6 +6,7 @@
         "danger": "^11.2.3",
         "axios": "^1.3.3",
         "langchain": "^0.0.53",
-        "openai-gpt-token-counter": "^1.0.3"
+        "openai-gpt-token-counter": "^1.0.3",
+        "@commitlint/lint": "^13.1.0"
     }
 }

+ 5 - 0
.pre-commit-config.yaml

@@ -168,3 +168,8 @@ repos:
     hooks:
       - id: check-copyright
         args: ['--ignore', 'tools/ci/check_copyright_ignore.txt', '--config', 'tools/ci/check_copyright_config.yaml']
+  - repo: https://github.com/espressif/conventional-precommit-linter
+    rev: v1.0.0
+    hooks:
+      - id: conventional-precommit-linter
+        stages: [commit-msg]

Vissa filer visades inte eftersom för många filer har ändrats