From e8b3b7675691b39d7280037a04d364ee776bd2d7 Mon Sep 17 00:00:00 2001 From: Lere Williams Date: Thu, 5 Oct 2023 22:44:59 -0700 Subject: [PATCH] Fix config defaulting --- badges/coverage.svg | 2 +- dist/config/schema.json | 9 +--- dist/index.js | 41 +++++++++++++------ src/config/schema.json | 9 +--- src/configuration.ts | 91 ++++++++++++++++++++++++++++------------- src/main.ts | 51 ++++++++++++++--------- 6 files changed, 125 insertions(+), 78 deletions(-) diff --git a/badges/coverage.svg b/badges/coverage.svg index 8786dd0..919d454 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1 +1 @@ -Coverage: 13.54%Coverage13.54% \ No newline at end of file +Coverage: 15.15%Coverage15.15% \ No newline at end of file diff --git a/dist/config/schema.json b/dist/config/schema.json index 1307def..4e81db2 100644 --- a/dist/config/schema.json +++ b/dist/config/schema.json @@ -35,12 +35,5 @@ "sizeup": { "$ref": "https://raw.githubusercontent.com/lerebear/sizeup/main/src/config/schema.json" } - }, - "required": [ - "applyCategoryLabels", - "categoryLabelPrefix", - "addCommentWhenScoreThresholdHasBeenExceeded", - "scoreThresholdExceededCommentTemplate", - "sizeup" - ] + } } diff --git a/dist/index.js b/dist/index.js index ee94cab..cbb7aae 100644 --- a/dist/index.js +++ b/dist/index.js @@ -10601,6 +10601,14 @@ const sizeup_1 = __importDefault(__nccwpck_require__(2103)); const YAML = __importStar(__nccwpck_require__(4083)); const fs = __importStar(__nccwpck_require__(7147)); const path = __importStar(__nccwpck_require__(1017)); +const DEFAULT_LABEL_PREFIX = 'sizeup/'; +const DEFAULT_COMMENT_TEMPLATE = ` +👋 @{{author}} this pull request exceeds the configured reviewability score threshold of {{threshold}}. Your actual score was {{score}}. + +[Research](https://www.cabird.com/static/93aba3256c80506d3948983db34d3ba3/rigby2013convergent.pdf) has shown that this makes it harder for reviewers to provide quality feedback. + +We recommend that you reduce the size of this PR by separating commits into stacked PRs. +`; /** * The main function for the action. * @@ -10646,9 +10654,7 @@ function loadConfiguration() { core.info('Using default sizeup configuration'); configFile = path.resolve(__dirname, './config/default.yaml'); } - const config = fs.readFileSync(configFile, 'utf8'); - core.debug(`Retrieved config: ${config}`); - return YAML.parse(config); + return YAML.parse(fs.readFileSync(configFile, 'utf8')); } /** * Uses the `sizeup` library to evalute the given pull request for reviewability @@ -10664,19 +10670,22 @@ async function evaluatePullRequest(pull, config) { const pullRequestNickname = `${pull.base.repo.owner.login}/${pull.base.repo.name}#${pull.number}`; core.info(`Evaluating pull request ${pullRequestNickname}`); const octokit = github.getOctokit(core.getInput('token')); - const sizeupConfigFile = path.resolve(__dirname, './tmp/sizeup.yaml'); const diff = (await octokit.rest.pulls.get({ owner: pull.base.repo.owner.login, repo: pull.base.repo.name, pull_number: pull.number, mediaType: { format: 'diff' } })).data; - fs.mkdirSync(path.dirname(sizeupConfigFile)); - const yaml = YAML.stringify(config.sizeup); - core.debug(`Writing to ${sizeupConfigFile} with ${yaml} derived from ${JSON.stringify(config.sizeup)}`); - fs.writeFileSync(sizeupConfigFile, yaml); + let sizeupConfigFile = undefined; + if (config.sizeup) { + sizeupConfigFile = path.resolve(__dirname, './tmp/sizeup.yaml'); + fs.mkdirSync(path.dirname(sizeupConfigFile)); + fs.writeFileSync(sizeupConfigFile, YAML.stringify(config.sizeup)); + } const score = sizeup_1.default.evaluate(diff, sizeupConfigFile); - fs.rmSync(sizeupConfigFile, { force: true, recursive: true }); + if (sizeupConfigFile) { + fs.rmSync(sizeupConfigFile, { force: true, recursive: true }); + } core.info(`${pullRequestNickname} received a score of ${score.result} (${score.category.name})`); core.info(`The score was computed as follows:\n${score.toString()}`); core.setOutput('score', score.result); @@ -10693,7 +10702,7 @@ async function evaluatePullRequest(pull, config) { */ async function applyCategoryLabel(pull, score, config) { if (!score.category?.label) { - core.info('Skipping labelling because no category label was provided'); + core.info('Skipping labeling because no category label was provided'); return; } await ensureLabelExists(pull, score, config); @@ -10745,14 +10754,17 @@ async function ensureLabelExists(pull, score, config) { */ async function applyLabel(pull, score, config) { const octokit = github.getOctokit(core.getInput('token')); - const newLabelName = `${config.categoryLabelPrefix}${score.category.label.name}`; + const labelPrefix = config.categoryLabelPrefix !== undefined + ? config.categoryLabelPrefix + : DEFAULT_LABEL_PREFIX; + const newLabelName = `${labelPrefix}${score.category.label.name}`; const labelsToAdd = [newLabelName]; const labelsToRemove = []; for (const existingLabel of pull.labels) { if (existingLabel.name === newLabelName) { labelsToAdd.pop(); } - else if (existingLabel.name.startsWith(config.categoryLabelPrefix)) { + else if (existingLabel.name.startsWith(labelPrefix)) { labelsToRemove.push(existingLabel.name); } } @@ -10802,7 +10814,10 @@ async function applyLabel(pull, score, config) { async function addScoreThresholdExceededComment(pull, score, config) { if (score.result <= score.threshold) return; - const comment = config.scoreThresholdExceededCommentTemplate + const commentTemplate = config.scoreThresholdExceededCommentTemplate !== undefined + ? config.scoreThresholdExceededCommentTemplate + : DEFAULT_COMMENT_TEMPLATE; + const comment = commentTemplate .replaceAll('{{author}}', pull.user.login) .replaceAll('{{score}}', `${score.result}`) .replaceAll('{{category}}', score.category.name) diff --git a/src/config/schema.json b/src/config/schema.json index 1307def..4e81db2 100644 --- a/src/config/schema.json +++ b/src/config/schema.json @@ -35,12 +35,5 @@ "sizeup": { "$ref": "https://raw.githubusercontent.com/lerebear/sizeup/main/src/config/schema.json" } - }, - "required": [ - "applyCategoryLabels", - "categoryLabelPrefix", - "addCommentWhenScoreThresholdHasBeenExceeded", - "scoreThresholdExceededCommentTemplate", - "sizeup" - ] + } } diff --git a/src/configuration.ts b/src/configuration.ts index bcc2368..64f23fa 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -11,26 +11,26 @@ export interface Configuration { /** * Whether or not to apply the score category label to each assessed pull request */ - applyCategoryLabels: boolean + applyCategoryLabels?: boolean /** * The prefix to add to each category label that we apply */ - categoryLabelPrefix: string + categoryLabelPrefix?: string /** * Whether or not to add a comment to each assessed pull request that exceeds the configured score threshold */ - addCommentWhenScoreThresholdHasBeenExceeded: boolean + addCommentWhenScoreThresholdHasBeenExceeded?: boolean /** * The template for the comment that should be added to each pull request that exceeds the configured score threshold */ - scoreThresholdExceededCommentTemplate: string + scoreThresholdExceededCommentTemplate?: string /** * A list of GitHub handles for users or teams that have opted into this workflow * * @minItems 1 */ optIns?: [string, ...string[]] - sizeup: Configuration1 + sizeup?: Configuration1 } /** * YAML configuration accepted by sizeup @@ -41,37 +41,70 @@ export interface Configuration1 { * * @minItems 1 */ - categories?: { - /** - * human-friendly name of the category - */ - name: string - /** - * A visual label that should be used to represent this category - */ - label?: { + categories?: [ + { /** - * name of the label that should be used to represent this category + * human-friendly name of the category */ name: string /** - * describes the meaning of the label that will be used to represent this category + * A visual label that should be used to represent this category */ - description?: string + label?: { + /** + * name of the label that should be used to represent this category + */ + name: string + /** + * describes the meaning of the label that will be used to represent this category + */ + description?: string + /** + * preferred CSS hex color label that should be used to represent this category + */ + color?: string + } /** - * preferred CSS hex color label that should be used to represent this category + * inclusive upper bound on the score that a pull request must have to be assigned this category */ - color?: string - } - /** - * inclusive upper bound on the score that a pull request must have to be assigned this category - */ - lte?: number - /** - * Whether or not this category marks the threshold at which we should warn about the diff being difficult to review - */ - threshold?: boolean - }[] + lte?: number + /** + * Whether or not this category marks the threshold above which we should warn about the diff being difficult to review + */ + threshold?: boolean + }, + ...{ + /** + * human-friendly name of the category + */ + name: string + /** + * A visual label that should be used to represent this category + */ + label?: { + /** + * name of the label that should be used to represent this category + */ + name: string + /** + * describes the meaning of the label that will be used to represent this category + */ + description?: string + /** + * preferred CSS hex color label that should be used to represent this category + */ + color?: string + } + /** + * inclusive upper bound on the score that a pull request must have to be assigned this category + */ + lte?: number + /** + * Whether or not this category marks the threshold above which we should warn about the diff being difficult to review + */ + threshold?: boolean + }[] + ] scoring?: { /** * an expression, written in prefix-notation, that describes how to combine features to produce a score diff --git a/src/main.ts b/src/main.ts index 9f50cf8..757f0bd 100644 --- a/src/main.ts +++ b/src/main.ts @@ -8,6 +8,15 @@ import * as fs from 'fs' import * as path from 'path' import { Configuration } from './configuration' +const DEFAULT_LABEL_PREFIX = 'sizeup/' +const DEFAULT_COMMENT_TEMPLATE = ` +👋 @{{author}} this pull request exceeds the configured reviewability score threshold of {{threshold}}. Your actual score was {{score}}. + +[Research](https://www.cabird.com/static/93aba3256c80506d3948983db34d3ba3/rigby2013convergent.pdf) has shown that this makes it harder for reviewers to provide quality feedback. + +We recommend that you reduce the size of this PR by separating commits into stacked PRs. +` + /** * The main function for the action. * @@ -63,9 +72,7 @@ function loadConfiguration(): Configuration { configFile = path.resolve(__dirname, './config/default.yaml') } - const config = fs.readFileSync(configFile, 'utf8') - core.debug(`Retrieved config: ${config}`) - return YAML.parse(config) as Configuration + return YAML.parse(fs.readFileSync(configFile, 'utf8')) as Configuration } /** @@ -86,7 +93,6 @@ async function evaluatePullRequest( core.info(`Evaluating pull request ${pullRequestNickname}`) const octokit = github.getOctokit(core.getInput('token')) - const sizeupConfigFile = path.resolve(__dirname, './tmp/sizeup.yaml') const diff = ( await octokit.rest.pulls.get({ owner: pull.base.repo.owner.login, @@ -96,18 +102,18 @@ async function evaluatePullRequest( }) ).data as unknown as string - fs.mkdirSync(path.dirname(sizeupConfigFile)) - const yaml = YAML.stringify(config.sizeup) - core.debug( - `Writing to ${sizeupConfigFile} with ${yaml} derived from ${JSON.stringify( - config.sizeup - )}` - ) - fs.writeFileSync(sizeupConfigFile, yaml) + let sizeupConfigFile = undefined + if (config.sizeup) { + sizeupConfigFile = path.resolve(__dirname, './tmp/sizeup.yaml') + fs.mkdirSync(path.dirname(sizeupConfigFile)) + fs.writeFileSync(sizeupConfigFile, YAML.stringify(config.sizeup)) + } const score = SizeUp.evaluate(diff, sizeupConfigFile) - fs.rmSync(sizeupConfigFile, { force: true, recursive: true }) + if (sizeupConfigFile) { + fs.rmSync(sizeupConfigFile, { force: true, recursive: true }) + } core.info( `${pullRequestNickname} received a score of ${score.result} (${ @@ -135,7 +141,7 @@ async function applyCategoryLabel( config: Configuration ): Promise { if (!score.category?.label) { - core.info('Skipping labelling because no category label was provided') + core.info('Skipping labeling because no category label was provided') return } @@ -200,16 +206,18 @@ async function applyLabel( config: Configuration ): Promise { const octokit = github.getOctokit(core.getInput('token')) - const newLabelName = `${config.categoryLabelPrefix}${ - score.category!.label!.name - }` + const labelPrefix = + config.categoryLabelPrefix !== undefined + ? config.categoryLabelPrefix + : DEFAULT_LABEL_PREFIX + const newLabelName = `${labelPrefix}${score.category!.label!.name}` const labelsToAdd = [newLabelName] const labelsToRemove = [] for (const existingLabel of pull.labels) { if (existingLabel.name === newLabelName) { labelsToAdd.pop() - } else if (existingLabel.name.startsWith(config.categoryLabelPrefix)) { + } else if (existingLabel.name.startsWith(labelPrefix)) { labelsToRemove.push(existingLabel.name) } } @@ -264,7 +272,12 @@ async function addScoreThresholdExceededComment( ): Promise { if (score.result! <= score.threshold!) return - const comment = config.scoreThresholdExceededCommentTemplate + const commentTemplate = + config.scoreThresholdExceededCommentTemplate !== undefined + ? config.scoreThresholdExceededCommentTemplate + : DEFAULT_COMMENT_TEMPLATE + + const comment = commentTemplate .replaceAll('{{author}}', pull.user.login) .replaceAll('{{score}}', `${score.result!}`) .replaceAll('{{category}}', score.category!.name)