From 8bb8ca9d67375d0bd58ceacbc6346f9e92fef872 Mon Sep 17 00:00:00 2001 From: Mothblocks <35135081+Mothblocks@users.noreply.github.com> Date: Wed, 26 Oct 2022 20:52:32 -0700 Subject: [PATCH] Move auto changelog generation into its own workflow (#70652) I wanted to move auto labeling into its own workflow, but realized that it and changelog generation were coupled. So I'm detaching that first, and then will work on auto labeling later. Includes general code for a changelog parser so that I can reuse it by that point. GitHub actions are great for downstreams, as setting up a PHP server for the webhook is not trivial, compared to Existing and getting it for free. They are also much more straightforward to test and update than the webhook. I was able to verify this was working trivially with an empty repository. Tested and working: Mothblocks/ss13-workflow-testing@0a2de4d --- .github/CODEOWNERS | 1 + .github/workflows/auto_changelog.yml | 23 ++++ .../github_webhook_processor.php | 80 ++--------- tools/pull_request_hooks/autoChangelog.js | 42 ++++++ .../pull_request_hooks/autoChangelog.test.js | 21 +++ tools/pull_request_hooks/changelogConfig.js | 127 ++++++++++++++++++ tools/pull_request_hooks/changelogParser.js | 80 +++++++++++ .../changelogParser.test.js | 72 ++++++++++ tools/pull_request_hooks/package.json | 3 + 9 files changed, 381 insertions(+), 68 deletions(-) create mode 100644 .github/workflows/auto_changelog.yml create mode 100644 tools/pull_request_hooks/autoChangelog.js create mode 100644 tools/pull_request_hooks/autoChangelog.test.js create mode 100644 tools/pull_request_hooks/changelogConfig.js create mode 100644 tools/pull_request_hooks/changelogParser.js create mode 100644 tools/pull_request_hooks/changelogParser.test.js create mode 100644 tools/pull_request_hooks/package.json diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 496e05e233a..74bdd4be963 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -58,6 +58,7 @@ /code/modules/preferences_menu.dm @Mothblocks /code/modules/preferences_savefile.dm @Mothblocks /tgui/packages/tgui/interfaces/PreferencesMenu/ @Mothblocks +/tools/pull_request_hooks/ @Mothblocks /tools/screenshot-test-comparison/ @Mothblocks /tools/test_merge_bot/ @Mothblocks diff --git a/.github/workflows/auto_changelog.yml b/.github/workflows/auto_changelog.yml new file mode 100644 index 00000000000..cb7c073f5cb --- /dev/null +++ b/.github/workflows/auto_changelog.yml @@ -0,0 +1,23 @@ +# Creates an entry in html/changelogs automatically, to eventually be compiled by compile_changelogs +name: Auto Changelog +on: + pull_request_target: + types: + - closed + branches: + - master +permissions: + contents: write +jobs: + auto_labeler: + runs-on: ubuntu-latest + if: github.event.pull_request.merged == true + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Run auto labeler + uses: actions/github-script@v6 + with: + script: | + const { processAutoChangelog } = await import('${{ github.workspace }}/tools/pull_request_hooks/autoChangelog.js') + await processAutoChangelog({ github, context }) diff --git a/tools/WebhookProcessor/github_webhook_processor.php b/tools/WebhookProcessor/github_webhook_processor.php index 4ee4dba83c7..2c3f60ec9ad 100644 --- a/tools/WebhookProcessor/github_webhook_processor.php +++ b/tools/WebhookProcessor/github_webhook_processor.php @@ -240,7 +240,7 @@ function tag_pr($payload, $opened) { $tags = array(); $title = $payload['pull_request']['title']; if($opened) { //you only have one shot on these ones so as to not annoy maintainers - $tags = checkchangelog($payload, false); + $tags = checkchangelog($payload); if(strpos(strtolower($title), 'refactor') !== FALSE) $tags[] = 'Refactor'; @@ -302,7 +302,7 @@ function check_dismiss_changelog_review($payload){ return; if(!$no_changelog) - checkchangelog($payload, false); + checkchangelog($payload); $review_message = 'Your changelog for this PR is either malformed or non-existent. Please create one to document your changes.'; @@ -362,7 +362,7 @@ function handle_pr($payload) { else { $action = 'merged'; auto_update($payload); - checkchangelog($payload, true); + checkchangelog($payload); $validated = TRUE; //pr merged events always get announced. } break; @@ -630,7 +630,7 @@ function has_tree_been_edited($payload, $tree){ } $no_changelog = false; -function checkchangelog($payload, $compile = true) { +function checkchangelog($payload) { global $no_changelog; if (!isset($payload['pull_request']) || !isset($payload['pull_request']['body'])) { return; @@ -648,26 +648,16 @@ function checkchangelog($payload, $compile = true) { $body = str_replace("\r\n", "\n", $body); $body = explode("\n", $body); - $username = $payload['pull_request']['user']['login']; $incltag = false; - $changelogbody = array(); - $currentchangelogblock = array(); $foundcltag = false; foreach ($body as $line) { $line = trim($line); if (substr($line,0,4) == ':cl:' || substr($line,0,1) == '??') { $incltag = true; $foundcltag = true; - $pos = strpos($line, " "); - if ($pos) { - $tmp = substr($line, $pos+1); - if (trim($tmp) != 'optional name here') - $username = $tmp; - } continue; } else if (substr($line,0,5) == '/:cl:' || substr($line,0,6) == '/ :cl:' || substr($line,0,5) == ':/cl:' || substr($line,0,5) == '/??' || substr($line,0,6) == '/ ??' ) { $incltag = false; - $changelogbody = array_merge($changelogbody, $currentchangelogblock); continue; } if (!$incltag) @@ -683,47 +673,44 @@ function checkchangelog($payload, $compile = true) { $firstword = $line; } + // Line is empty if (!strlen($firstword)) { - if (count($currentchangelogblock) <= 0) - continue; - $currentchangelogblock[count($currentchangelogblock)-1]['body'] .= "\n"; continue; } + //not a prefix line. - //so we add it to the last changelog entry as a separate line if (!strlen($firstword) || $firstword[strlen($firstword)-1] != ':') { - if (count($currentchangelogblock) <= 0) - continue; - $currentchangelogblock[count($currentchangelogblock)-1]['body'] .= "\n".$line; continue; } + $cltype = strtolower(substr($firstword, 0, -1)); + + // !!! + // !!! If you are changing any of these at the bottom, also edit `tools/pull_request_hooks/changelogConfig.js`. + // !!! + switch ($cltype) { case 'fix': case 'fixes': case 'bugfix': if($item != 'fixed a few things') { $tags[] = 'Fix'; - $currentchangelogblock[] = array('type' => 'bugfix', 'body' => $item); } break; case 'qol': if($item != 'made something easier to use') { $tags[] = 'Quality of Life'; - $currentchangelogblock[] = array('type' => 'qol', 'body' => $item); } break; case 'soundadd': if($item != 'added a new sound thingy') { $tags[] = 'Sound'; - $currentchangelogblock[] = array('type' => 'soundadd', 'body' => $item); } break; case 'sounddel': if($item != 'removed an old sound thingy') { $tags[] = 'Sound'; $tags[] = 'Removal'; - $currentchangelogblock[] = array('type' => 'sounddel', 'body' => $item); } break; case 'add': @@ -731,7 +718,6 @@ function checkchangelog($payload, $compile = true) { case 'rscadd': if($item != 'Added new mechanics or gameplay changes' && $item != 'Added more things') { $tags[] = 'Feature'; - $currentchangelogblock[] = array('type' => 'rscadd', 'body' => $item); } break; case 'del': @@ -739,95 +725,53 @@ function checkchangelog($payload, $compile = true) { case 'rscdel': if($item != 'Removed old things') { $tags[] = 'Removal'; - $currentchangelogblock[] = array('type' => 'rscdel', 'body' => $item); } break; case 'imageadd': if($item != 'added some icons and images') { $tags[] = 'Sprites'; - $currentchangelogblock[] = array('type' => 'imageadd', 'body' => $item); } break; case 'imagedel': if($item != 'deleted some icons and images') { $tags[] = 'Sprites'; $tags[] = 'Removal'; - $currentchangelogblock[] = array('type' => 'imagedel', 'body' => $item); } break; case 'typo': case 'spellcheck': if($item != 'fixed a few typos') { $tags[] = 'Grammar and Formatting'; - $currentchangelogblock[] = array('type' => 'spellcheck', 'body' => $item); } break; case 'balance': if($item != 'rebalanced something'){ $tags[] = 'Balance'; - $currentchangelogblock[] = array('type' => 'balance', 'body' => $item); } break; case 'code_imp': case 'code': if($item != 'changed some code'){ $tags[] = 'Code Improvement'; - $currentchangelogblock[] = array('type' => 'code_imp', 'body' => $item); } break; case 'refactor': if($item != 'refactored some code'){ $tags[] = 'Refactor'; - $currentchangelogblock[] = array('type' => 'refactor', 'body' => $item); } break; case 'config': if($item != 'changed some config setting'){ $tags[] = 'Config Update'; - $currentchangelogblock[] = array('type' => 'config', 'body' => $item); } break; case 'admin': if($item != 'messed with admin stuff'){ $tags[] = 'Administration'; - $currentchangelogblock[] = array('type' => 'admin', 'body' => $item); } break; - case 'server': - if($item != 'something server ops should know') - $currentchangelogblock[] = array('type' => 'server', 'body' => $item); - break; - default: - //we add it to the last changelog entry as a separate line - if (count($currentchangelogblock) > 0) - $currentchangelogblock[count($currentchangelogblock)-1]['body'] .= "\n".$line; - break; } } - - if(!count($changelogbody)) - $no_changelog = true; - - if ($no_changelog || !$compile) - return $tags; - - $file = 'author: "'.trim(str_replace(array("\\", '"'), array("\\\\", "\\\""), $username)).'"'."\n"; - $file .= "delete-after: True\n"; - $file .= "changes: \n"; - foreach ($changelogbody as $changelogitem) { - $type = $changelogitem['type']; - $body = trim(str_replace(array("\\", '"'), array("\\\\", "\\\""), $changelogitem['body'])); - $file .= ' - '.$type.': "'.$body.'"'; - $file .= "\n"; - } - $content = array ( - 'branch' => $payload['pull_request']['base']['ref'], - 'message' => 'Automatic changelog generation for PR #'.$payload['pull_request']['number'].' [ci skip]', - 'content' => base64_encode($file) - ); - - $filename = '/html/changelogs/AutoChangeLog-pr-'.$payload['pull_request']['number'].'.yml'; - echo github_apisend($payload['pull_request']['base']['repo']['url'].'/contents'.$filename, 'PUT', $content); } function game_server_send($addr, $port, $str) { diff --git a/tools/pull_request_hooks/autoChangelog.js b/tools/pull_request_hooks/autoChangelog.js new file mode 100644 index 00000000000..bc24481f182 --- /dev/null +++ b/tools/pull_request_hooks/autoChangelog.js @@ -0,0 +1,42 @@ +import { parseChangelog } from "./changelogParser.js"; + +const safeYml = (string) => + string.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\n/g, "\\n"); + +export function changelogToYml(changelog, login) { + const author = changelog.author || login; + const ymlLines = []; + + ymlLines.push(`author: "${safeYml(author)}"`); + ymlLines.push(`delete-after: True`); + ymlLines.push(`changes:`); + + for (const change of changelog.changes) { + ymlLines.push( + ` - ${change.type.changelogKey}: "${safeYml(change.description)}"` + ); + } + + return ymlLines.join("\n"); +} + +export async function processAutoChangelog({ github, context }) { + const changelog = parseChangelog(context.payload.pull_request.body); + if (!changelog || changelog.changes.length === 0) { + console.log("no changelog found"); + return; + } + + const yml = changelogToYml( + changelog, + context.payload.pull_request.user.login + ); + + github.rest.repos.createOrUpdateFileContents({ + owner: context.repo.owner, + repo: context.repo.repo, + path: `html/changelogs/AutoChangeLog-pr-${context.payload.pull_request.number}.yml`, + message: `Automatic changelog for PR #${context.payload.pull_request.number} [ci skip]`, + content: Buffer.from(yml).toString("base64"), + }); +} diff --git a/tools/pull_request_hooks/autoChangelog.test.js b/tools/pull_request_hooks/autoChangelog.test.js new file mode 100644 index 00000000000..e7c29c384a9 --- /dev/null +++ b/tools/pull_request_hooks/autoChangelog.test.js @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import { changelogToYml } from "./autoChangelog.js"; +import { parseChangelog } from "./changelogParser.js"; + +assert.equal( + changelogToYml( + parseChangelog(` + My cool PR! + :cl: DenverCoder9 + add: Adds new stuff + add: Adds more stuff + /:cl: + `) + ), + + `author: "DenverCoder9" +delete-after: True +changes: + - rscadd: "Adds new stuff" + - rscadd: "Adds more stuff"` +); diff --git a/tools/pull_request_hooks/changelogConfig.js b/tools/pull_request_hooks/changelogConfig.js new file mode 100644 index 00000000000..a607fd9a457 --- /dev/null +++ b/tools/pull_request_hooks/changelogConfig.js @@ -0,0 +1,127 @@ +/** + * A map of changelog phrases to meta-information. + * + * The first entry in the list is used in the changelog YML file as the key when + * used, but other than that all entries are equivalent. + * + * placeholders - The default messages, if the changelog has this then we pretend it + * doesn't exist. + */ +export const CHANGELOG_ENTRIES = [ + [ + ["rscadd", "add", "adds"], + { + placeholders: [ + "Added new mechanics or gameplay changes", + "Added more things", + ], + }, + ], + + [ + ["bugfix", "fix", "fixes"], + { + placeholders: ["fixed a few things"], + }, + ], + + [ + ["rscdel", "del", "dels"], + { + placeholders: ["Removed old things"], + }, + ], + + [ + ["qol"], + { + placeholders: ["made something easier to use"], + }, + ], + + [ + ["soundadd"], + { + placeholders: ["added a new sound thingy"], + }, + ], + + [ + ["sounddel"], + { + placeholders: ["removed an old sound thingy"], + }, + ], + + [ + ["imageadd"], + { + placeholders: ["added some icons and images"], + }, + ], + + [ + ["imagedel"], + { + placeholders: ["deleted some icons and images"], + }, + ], + + [ + ["spellcheck", "typo"], + { + placeholders: ["fixed a few typos"], + }, + ], + + [ + ["balance"], + { + placeholders: ["rebalanced something"], + }, + ], + + [ + ["code_imp", "code"], + { + placeholders: ["changed some code"], + }, + ], + + [ + ["refactor"], + { + placeholders: ["refactored some code"], + }, + ], + + [ + ["config"], + { + placeholders: ["changed some config setting"], + }, + ], + + [ + ["admin"], + { + placeholders: ["messed with admin stuff"], + }, + ], + + [ + ["server"], + { + placeholders: ["something server ops should know"], + }, + ], +]; + +// Valid changelog openers +export const CHANGELOG_OPEN_TAGS = [":cl:", "??"]; + +// Valid changelog closers +export const CHANGELOG_CLOSE_TAGS = ["/:cl:", "/ :cl:", ":/cl:", "/??", "/ ??"]; + +// Placeholder value for an author +export const CHANGELOG_AUTHOR_PLACEHOLDER_NAME = "optional name here"; diff --git a/tools/pull_request_hooks/changelogParser.js b/tools/pull_request_hooks/changelogParser.js new file mode 100644 index 00000000000..2daa2a57989 --- /dev/null +++ b/tools/pull_request_hooks/changelogParser.js @@ -0,0 +1,80 @@ +import * as changelogConfig from "./changelogConfig.js"; + +const REGEX_CHANGELOG_LINE = /^(\w+): (.+)$/; + +const CHANGELOG_KEYS_TO_ENTRY = {}; +for (const [types, entry] of changelogConfig.CHANGELOG_ENTRIES) { + const entryWithChangelogKey = { + ...entry, + changelogKey: types[0], + }; + + for (const type of types) { + CHANGELOG_KEYS_TO_ENTRY[type] = entryWithChangelogKey; + } +} + +function parseChangelogBody(lines, openTag) { + const [changelogOpening] = lines.splice(0, 1); + + const author = + changelogOpening.substring(openTag.length).trim() || undefined; + + const changelog = { + author, + changes: [], + }; + + for (const line of lines) { + if (line.trim().length === 0) { + continue; + } + + for (const closeTag of changelogConfig.CHANGELOG_CLOSE_TAGS) { + if (line.startsWith(closeTag)) { + return changelog; + } + } + + const match = line.match(REGEX_CHANGELOG_LINE); + if (match) { + const [_, type, description] = match; + + const entry = CHANGELOG_KEYS_TO_ENTRY[type]; + + if (entry.placeholders.includes(description)) { + continue; + } + + if (entry) { + changelog.changes.push({ + type: entry, + description, + }); + } + } else { + const lastChange = changelog.changes[changelog.changes.length - 1]; + if (lastChange) { + lastChange.description += `\n${line}`; + } + } + } + + return changelog; +} + +export function parseChangelog(text) { + const lines = text.split("\n").map((line) => line.trim()); + + for (let index = 0; index < lines.length; index++) { + const line = lines[index]; + + for (const openTag of changelogConfig.CHANGELOG_OPEN_TAGS) { + if (line.startsWith(openTag)) { + return parseChangelogBody(lines.slice(index), openTag); + } + } + } + + return undefined; +} diff --git a/tools/pull_request_hooks/changelogParser.test.js b/tools/pull_request_hooks/changelogParser.test.js new file mode 100644 index 00000000000..bcf044ee36d --- /dev/null +++ b/tools/pull_request_hooks/changelogParser.test.js @@ -0,0 +1,72 @@ +import { strict as assert } from "node:assert"; +import { parseChangelog } from "./changelogParser.js"; + +// Basic test +const basicChangelog = parseChangelog(` + My cool PR! + :cl: DenverCoder9 + add: Adds new stuff + /:cl: +`); + +assert.equal(basicChangelog.author, "DenverCoder9"); +assert.equal(basicChangelog.changes.length, 1); +assert.equal(basicChangelog.changes[0].type.changelogKey, "rscadd"); +assert.equal(basicChangelog.changes[0].description, "Adds new stuff"); + +// Multi-line test +const multiLineChangelog = parseChangelog(` + My cool PR! + :cl: + add: Adds new stuff + to the game + /:cl: +`); + +assert.equal(multiLineChangelog.author, undefined); +assert.equal(multiLineChangelog.changes.length, 1); +assert.equal(multiLineChangelog.changes[0].type.changelogKey, "rscadd"); +assert.equal( + multiLineChangelog.changes[0].description, + "Adds new stuff\nto the game" +); + +// Placeholders +const placeholderChangelog = parseChangelog(` + My cool PR! + :cl: + add: Added new mechanics or gameplay changes + /:cl: +`); + +assert.equal(placeholderChangelog.changes.length, 0); + +// No changelog +const noChangelog = parseChangelog(` + My cool PR! +`); + +assert.equal(noChangelog, undefined); + +// No /:cl: + +const noCloseChangelog = parseChangelog(` + My cool PR! + :cl: + add: Adds new stuff +`); + +assert.equal(noCloseChangelog.changes.length, 1); +assert.equal(noCloseChangelog.changes[0].type.changelogKey, "rscadd"); +assert.equal(noCloseChangelog.changes[0].description, "Adds new stuff"); + +// :cl: with arbitrary text + +const arbitraryTextChangelog = parseChangelog(` + My cool PR! + :cl: + Adds new stuff + /:cl: +`); + +assert.equal(arbitraryTextChangelog.changes.length, 0); diff --git a/tools/pull_request_hooks/package.json b/tools/pull_request_hooks/package.json new file mode 100644 index 00000000000..bedb411a912 --- /dev/null +++ b/tools/pull_request_hooks/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +}