Skip to content

Commit a063f08

Browse files
authored
ci: lintcommit.js regressions aws#5515
Problem: Since 8e34c5e, `node lintcommit.js test` fails various tests and CI fails for those cases too: /home/runner/work/aws-toolkit-vscode/aws-toolkit-vscode/.github/workflows/lintcommit.js:80 } else if (!scope && typeScope.includes('(')) { Solution: Revert the changes from 8e34c5e. It's good to avoid code duplication, but in this case `parsePRTitle()` is not the right abstraction: - it doesn't signal failures in a way that is handled by callers - its return type is awkward (`undefined | string | object`) - notify.js doesn't actually need `parsePRTitle`, it only needs to check `startsWith()`.
1 parent 78423ea commit a063f08

File tree

4 files changed

+17
-28
lines changed

4 files changed

+17
-28
lines changed

.github/workflows/lintcommit.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
//
1212

1313
const fs = require('fs')
14-
const { parsePRTitle } = require('./utils')
1514
// This script intentionally avoids github APIs so that:
1615
// 1. it is locally-debuggable
1716
// 2. the CI job is fast ("npm install" is slow)
@@ -69,7 +68,20 @@ void scopes
6968
* Returns undefined if `title` is valid, else an error message.
7069
*/
7170
function validateTitle(title) {
72-
const { type, scope, subject } = parsePRTitle(title)
71+
const parts = title.split(':')
72+
const subject = parts.slice(1).join(':').trim()
73+
74+
if (title.startsWith('Merge')) {
75+
return undefined
76+
}
77+
78+
if (parts.length < 2) {
79+
return 'missing colon (:) char'
80+
}
81+
82+
const typeScope = parts[0]
83+
84+
const [type, scope] = typeScope.split(/\(([^)]+)\)$/)
7385

7486
if (/\s+/.test(type)) {
7587
return `type contains whitespace: "${type}"`

.github/workflows/notification.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
name: Notifications
55

66
on:
7+
# `pull_request_target` (as opposed to `pull_request`) gives permissions to comment on PRs.
78
pull_request_target:
89
branches: [master, feature/*, staging]
910
# Default = opened + synchronize + reopened.

.github/workflows/notify.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
const { parsePRTitle, hasPath, dedupComment } = require('./utils')
6+
const { hasPath, dedupComment } = require('./utils')
77

88
const testFilesMessage =
99
'This pull request modifies files in src/ but no tests were added/updated. Confirm whether tests should be added or ensure the PR description explains why tests are not required.'
@@ -59,8 +59,7 @@ module.exports = async ({ github, context }) => {
5959
*/
6060
function requiresChangelogMessage(filenames, title) {
6161
try {
62-
const { type } = parsePRTitle(title)
63-
return !hasPath(filenames, '.changes') && (type === 'fix' || type === 'feat')
62+
return !hasPath(filenames, '.changes') && (title.startsWith('fix') || title.startsWith('feat'))
6463
} catch (e) {
6564
console.log(e)
6665
return undefined

.github/workflows/utils.js

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,6 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
function parsePRTitle(title) {
7-
const parts = title.split(':')
8-
const subject = parts.slice(1).join(':').trim()
9-
10-
if (title.startsWith('Merge')) {
11-
return undefined
12-
}
13-
14-
if (parts.length < 2) {
15-
return 'missing colon (:) char'
16-
}
17-
18-
const typeScope = parts[0]
19-
20-
const [type, scope] = typeScope.split(/\(([^)]+)\)$/)
21-
return {
22-
type,
23-
scope,
24-
subject,
25-
}
26-
}
27-
286
/**
297
* Create a comment on a PR if one does not already exist
308
*/
@@ -49,7 +27,6 @@ function hasPath(filenames, path) {
4927
}
5028

5129
module.exports = {
52-
parsePRTitle,
5330
dedupComment,
5431
hasPath,
5532
}

0 commit comments

Comments
 (0)