From 652e4a6318d8b58c625df4e7cec05adf852550b0 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 14 Feb 2023 10:40:20 +0100 Subject: [PATCH 1/5] commitlint/abbreviations: add 3 more --- commitlint/abbreviations.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/commitlint/abbreviations.ts b/commitlint/abbreviations.ts index b9e0695c..fc9ff887 100644 --- a/commitlint/abbreviations.ts +++ b/commitlint/abbreviations.ts @@ -65,6 +65,7 @@ export let abbr = { "convert": "conv", "coordinate": "coord", "coordinates": "coords", + "copy": "cp", "csharp": "C#", "c-sharp": "C#", "database": "DB", @@ -154,6 +155,7 @@ export let abbr = { "interfaces": "ifaces", "issue": "bug", "issues": "bugs", + "javascript": "JS", "language": "lang", "languages": "langs", "length": "len", @@ -279,6 +281,7 @@ export let abbr = { "values": "vals", "variable": "var", "variables": "vars", + "webassembly": "WebAsm", "yes/no": "y/n", "zero": "0", "zeroes": "0s", From 3e927accf027e55a5a10e6991292182c5f581202 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 14 Feb 2023 10:45:19 +0100 Subject: [PATCH 2/5] commitlint.config: refactor, less verbose code --- commitlint.config.ts | 69 +++++++++----------------------------------- 1 file changed, 14 insertions(+), 55 deletions(-) diff --git a/commitlint.config.ts b/commitlint.config.ts index f58f0e74..af12c79e 100644 --- a/commitlint.config.ts +++ b/commitlint.config.ts @@ -62,9 +62,8 @@ module.exports = { { rules: { "body-prose": ({ raw }: { raw: any }) => { - let rawUncastedStr = Helpers.convertAnyToString(raw, "raw"); let rawStr = Helpers.assertNotNull( - rawUncastedStr, + Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") ).trim(); @@ -72,9 +71,8 @@ module.exports = { }, "commit-hash-alone": ({ raw }: { raw: any }) => { - let rawUncastedStr = Helpers.convertAnyToString(raw, "raw"); let rawStr = Helpers.assertNotNull( - rawUncastedStr, + Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") ); @@ -82,12 +80,8 @@ module.exports = { }, "empty-wip": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -99,12 +93,8 @@ module.exports = { _: any, maxLineLength: number ) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -130,12 +120,8 @@ module.exports = { }: { header: any; }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -143,9 +129,8 @@ module.exports = { }, "proper-issue-refs": ({ raw }: { raw: any }) => { - let rawUncastedStr = Helpers.convertAnyToString(raw, "raw"); let rawStr = Helpers.assertNotNull( - rawUncastedStr, + Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") ).trim(); @@ -153,12 +138,8 @@ module.exports = { }, "title-uppercase": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -166,9 +147,8 @@ module.exports = { }, "too-many-spaces": ({ raw }: { raw: any }) => { - let rawUncastedStr = Helpers.convertAnyToString(raw, "raw"); let rawStr = Helpers.assertNotNull( - rawUncastedStr, + Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") ); @@ -176,12 +156,8 @@ module.exports = { }, "type-space-after-colon": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -189,12 +165,8 @@ module.exports = { }, "type-with-square-brackets": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -203,24 +175,16 @@ module.exports = { // NOTE: we use 'header' instead of 'subject' as a workaround to this bug: https://github.com/conventional-changelog/commitlint/issues/3404 "subject-lowercase": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); return Plugins.subjectLowercase(headerStr); }, "type-space-after-comma": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); @@ -240,9 +204,8 @@ module.exports = { }, "trailing-whitespace": ({ raw }: { raw: any }) => { - let rawUncastedStr = Helpers.convertAnyToString(raw, "raw"); let rawStr = Helpers.assertNotNull( - rawUncastedStr, + Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") ); @@ -250,12 +213,8 @@ module.exports = { }, "type-space-before-paren": ({ header }: { header: any }) => { - let headerUncastedStr = Helpers.convertAnyToString( - header, - "header" - ); let headerStr = Helpers.assertNotNull( - headerUncastedStr, + Helpers.convertAnyToString(header, "header"), notNullStringErrorMessage("header") ); From 01b0ecfa5c893e07a2325a0b108214e28e7e9e38 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 14 Feb 2023 10:47:26 +0100 Subject: [PATCH 3/5] commitlint: refactor, calling trim() inside plugin It is the responsibility of the plugins to deal with rawStr in any way they want/need, not the caller of the plugin. --- commitlint.config.ts | 4 ++-- commitlint/plugins.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/commitlint.config.ts b/commitlint.config.ts index af12c79e..7cd76886 100644 --- a/commitlint.config.ts +++ b/commitlint.config.ts @@ -65,7 +65,7 @@ module.exports = { let rawStr = Helpers.assertNotNull( Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") - ).trim(); + ); return Plugins.bodyProse(rawStr); }, @@ -132,7 +132,7 @@ module.exports = { let rawStr = Helpers.assertNotNull( Helpers.convertAnyToString(raw, "raw"), notNullStringErrorMessage("raw") - ).trim(); + ); return Plugins.properIssueRefs(rawStr); }, diff --git a/commitlint/plugins.ts b/commitlint/plugins.ts index b4745b58..250ed920 100644 --- a/commitlint/plugins.ts +++ b/commitlint/plugins.ts @@ -5,6 +5,7 @@ export abstract class Plugins { public static bodyProse(rawStr: string) { let offence = false; + rawStr = rawStr.trim(); let lineBreakIndex = rawStr.indexOf("\n"); if (lineBreakIndex >= 0) { @@ -243,6 +244,7 @@ export abstract class Plugins { public static properIssueRefs(rawStr: string) { let offence = false; + rawStr = rawStr.trim(); let lineBreakIndex = rawStr.indexOf("\n"); if (lineBreakIndex >= 0) { From cf7b1bee2acc30ce89e3852199563bb6acb3453d Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 14 Feb 2023 11:15:57 +0100 Subject: [PATCH 4/5] commitlint: separate tests in 2 files Strictly speaking, there's one test that is checking that one of the commitlint's native rules works (not a plugin of ours). --- commitlint/integration.test.ts | 14 ++++++++++++++ commitlint/plugins.test.ts | 7 ------- 2 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 commitlint/integration.test.ts diff --git a/commitlint/integration.test.ts b/commitlint/integration.test.ts new file mode 100644 index 00000000..e9e82d9e --- /dev/null +++ b/commitlint/integration.test.ts @@ -0,0 +1,14 @@ +let cp = require("child_process"); + +function runCommitLintOnMsg(inputMsg: string) { + return cp.spawnSync("npx", ["commitlint", "--verbose"], { + input: inputMsg, + }); +} + +test("body-leading-blank1", () => { + let commitMsgWithoutEmptySecondLine = + "foo: this is only a title" + "\n" + "Bar baz."; + let bodyLeadingBlank1 = runCommitLintOnMsg(commitMsgWithoutEmptySecondLine); + expect(bodyLeadingBlank1.status).not.toBe(0); +}); diff --git a/commitlint/plugins.test.ts b/commitlint/plugins.test.ts index 51ba35f6..4d930283 100644 --- a/commitlint/plugins.test.ts +++ b/commitlint/plugins.test.ts @@ -4,13 +4,6 @@ function runCommitLintOnMsg(inputMsg: string) { return spawnSync("npx", ["commitlint", "--verbose"], { input: inputMsg }); } -test("body-leading-blank1", () => { - let commitMsgWithoutEmptySecondLine = - "foo: this is only a title" + "\n" + "Bar baz."; - let bodyLeadingBlank1 = runCommitLintOnMsg(commitMsgWithoutEmptySecondLine); - expect(bodyLeadingBlank1.status).not.toBe(0); -}); - test("body-prose1", () => { let commitMsgWithLowercaseBodyStart = "foo: this is only a title" + "\n\n" + "bla blah bla."; From 677a9732a6c79e684aad8c90c526e4f16504d7b7 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Tue, 14 Feb 2023 13:39:10 +0100 Subject: [PATCH 5/5] commitlint: subject-full-stop integration tests This rule was most of the time not working for us before, because of an upstream bug[1] which we have now fixed [2]. Given that a recent commit[3] upgraded our version of commitlint, we can just add integration tests now and we are done. [1] https://github.com/conventional-changelog/commitlint/issues/3530 [2] https://github.com/conventional-changelog/commitlint/pull/3531 [3] 1975174233f661d0adcd9799e7a8a5f656b19eb4 Co-authored-by: Zahra TehraniNasab --- commitlint.config.ts | 1 - commitlint/integration.test.ts | 12 ++++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/commitlint.config.ts b/commitlint.config.ts index 7cd76886..e404f0ce 100644 --- a/commitlint.config.ts +++ b/commitlint.config.ts @@ -54,7 +54,6 @@ module.exports = { // * Detect reverts which have not been elaborated. // * Reject some stupid obvious words: change, update, modify (if first word after colon, error; otherwise warning). // * Think of how to reject this shitty commit message: https://github.com/nblockchain/NOnion/pull/34/commits/9ffcb373a1147ed1c729e8aca4ffd30467255594 - // * Title should not have dot at the end. // * Workflow: detect if wip commit in a branch not named "wip/*" or whose name contains "squashed". // * Detect if commit hash mention in commit msg actually exists in repo. // * Detect scope(sub-scope) in the title that doesn't include scope part (e.g., writing (bar) instead of foo(bar)) diff --git a/commitlint/integration.test.ts b/commitlint/integration.test.ts index e9e82d9e..2dab17cb 100644 --- a/commitlint/integration.test.ts +++ b/commitlint/integration.test.ts @@ -12,3 +12,15 @@ test("body-leading-blank1", () => { let bodyLeadingBlank1 = runCommitLintOnMsg(commitMsgWithoutEmptySecondLine); expect(bodyLeadingBlank1.status).not.toBe(0); }); + +test("subject-full-stop1", () => { + let commitMsgWithEndingDotInTitle = "foo/bar: bla bla blah."; + let subjectFullStop1 = runCommitLintOnMsg(commitMsgWithEndingDotInTitle); + expect(subjectFullStop1.status).not.toBe(0); +}); + +test("subject-full-stop2", () => { + let commitMsgWithoutEndingDotInTitle = "foo/bar: bla bla blah"; + let subjectFullStop2 = runCommitLintOnMsg(commitMsgWithoutEndingDotInTitle); + expect(subjectFullStop2.status).toBe(0); +});