Skip to content

Commit 8e34c5e

Browse files
authored
ci(toolkit): add github comment for missing changelog item (aws#5499)
## Problem - If a user is using the types feat/fix in the PR title they should also include a changelog item ## Solution - Notify them if a changelog item isn't present
1 parent 3230739 commit 8e34c5e

File tree

3 files changed

+105
-34
lines changed

3 files changed

+105
-34
lines changed

.github/workflows/lintcommit.js

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

1313
const fs = require('fs')
14+
const { parsePRTitle } = require('./utils')
1415
// This script intentionally avoids github APIs so that:
1516
// 1. it is locally-debuggable
1617
// 2. the CI job is fast ("npm install" is slow)
@@ -68,20 +69,7 @@ void scopes
6869
* Returns undefined if `title` is valid, else an error message.
6970
*/
7071
function validateTitle(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(/\(([^)]+)\)$/)
72+
const { type, scope, subject } = parsePRTitle(title)
8573

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

.github/workflows/notify.js

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

6-
const needsTestFiles =
6+
const { parsePRTitle, hasPath, dedupComment } = require('./utils')
7+
8+
const testFilesMessage =
79
'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.'
810

11+
const changelogMessage =
12+
'This pull request modifies a feature or fixes a bug, but it does not include a changelog entry. All pull requests that introduce new features or bug fixes must have a corresponding changelog item describing the changes.'
13+
914
/**
1015
* Remind partner teams that tests are required. We don't need to remind them if:
1116
* 1. They did not change anything in a src directory
@@ -16,6 +21,7 @@ module.exports = async ({ github, context }) => {
1621
const owner = context.repo.owner
1722
const repo = context.repo.repo
1823
const author = context.payload.pull_request.head.repo.owner.login
24+
const pullRequestId = context.payload.pull_request.number
1925

2026
const response = await github.rest.repos.compareCommitsWithBasehead({
2127
owner,
@@ -25,36 +31,58 @@ module.exports = async ({ github, context }) => {
2531

2632
const filenames = response.data.files.map((file) => file.filename)
2733

28-
// Check if src directory changed
29-
const srcFiles = filenames.filter((file) => file.includes('src/'))
30-
if (srcFiles.length === 0) {
31-
console.log('Did not find src files in the code changes')
32-
return
33-
}
34+
const shouldAddTestFileMessage = requiresTestFilesMessage(filenames)
35+
const shouldAddChangelogMessage = requiresChangelogMessage(filenames, context.payload.pull_request.title)
3436

35-
// Check if test files were added or modified
36-
const testFiles = filenames.filter((file) => file.endsWith('.test.ts'))
37-
if (testFiles.length > 0) {
38-
console.log('Found test files in the code changes')
37+
if (!shouldAddTestFileMessage && !shouldAddChangelogMessage) {
3938
return
4039
}
4140

4241
// Check for prior comments on the PR
4342
const comments = await github.rest.issues.listComments({
4443
owner,
4544
repo,
46-
issue_number: context.payload.pull_request.number,
45+
issue_number: pullRequestId,
4746
})
4847

49-
if (comments.data.some((comment) => comment.body.includes(needsTestFiles))) {
50-
console.log('Found prior comment indicating tests are needed')
48+
if (shouldAddTestFileMessage) {
49+
await dedupComment({ github, comments, owner, repo, pullRequestId, message: testFilesMessage })
50+
}
51+
52+
if (shouldAddChangelogMessage) {
53+
await dedupComment({ github, comments, owner, repo, pullRequestId, message: changelogMessage })
54+
}
55+
}
56+
57+
/**
58+
* Require the changelog message if the scope is fix/feat AND there is no changelog item
59+
*/
60+
function requiresChangelogMessage(filenames, title) {
61+
try {
62+
const { type } = parsePRTitle(title)
63+
return !hasPath(filenames, '.changes') && (type === 'fix' || type === 'feat')
64+
} catch (e) {
65+
console.log(e)
66+
return undefined
67+
}
68+
}
69+
70+
/**
71+
* Require the test files message if there are changes to source files but aren't any
72+
* changes to the test files
73+
*/
74+
function requiresTestFilesMessage(filenames) {
75+
// Check if src directory changed
76+
if (!hasPath(filenames, 'src/')) {
77+
console.log('Did not find src files in the code changes')
5178
return
5279
}
5380

54-
await github.rest.issues.createComment({
55-
issue_number: context.issue.number,
56-
owner,
57-
repo,
58-
body: needsTestFiles,
59-
})
81+
// Check if test files were added or modified
82+
if (hasPath(filenames, '.test.ts')) {
83+
console.log('Found test files in the code changes')
84+
return
85+
}
86+
87+
return true
6088
}

.github/workflows/utils.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
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+
28+
/**
29+
* Create a comment on a PR if one does not already exist
30+
*/
31+
async function dedupComment({ github, pullRequestId, owner, repo, comments, message }) {
32+
if (comments.data.some((comment) => comment.body.includes(message))) {
33+
return
34+
}
35+
36+
await github.rest.issues.createComment({
37+
issue_number: pullRequestId,
38+
owner,
39+
repo,
40+
body: message,
41+
})
42+
}
43+
44+
/*
45+
* Check if path is included in at least one of the filename paths
46+
*/
47+
function hasPath(filenames, path) {
48+
return filenames.some((file) => file.includes(path))
49+
}
50+
51+
module.exports = {
52+
parsePRTitle,
53+
dedupComment,
54+
hasPath,
55+
}

0 commit comments

Comments
 (0)