From 73417c5792836e8b5d6fab7972202557151445fd Mon Sep 17 00:00:00 2001 From: Geoffrey Testelin Date: Sun, 23 May 2021 20:22:16 +0200 Subject: [PATCH] feat(options): simplify config by removing skip stale message options Closes #405 Closes #455 BREAKING CHANGES: remove skip-stale-issue-message and skip-stale-pr-message options. If you used this option, replace it by an empty message for the options stale-issue-message and stale-pr-message --- .../constants/default-processor-options.ts | 2 - __tests__/main.spec.ts | 56 +++++++++++++------ src/classes/issue.spec.ts | 2 - src/classes/issues-processor.ts | 18 +----- src/enums/option.ts | 2 - src/interfaces/issues-processor-options.ts | 2 - src/main.ts | 2 - 7 files changed, 41 insertions(+), 43 deletions(-) diff --git a/__tests__/constants/default-processor-options.ts b/__tests__/constants/default-processor-options.ts index 3b87f1e96..a0550f4b6 100644 --- a/__tests__/constants/default-processor-options.ts +++ b/__tests__/constants/default-processor-options.ts @@ -30,8 +30,6 @@ export const DefaultProcessorOptions: IIssuesProcessorOptions = Object.freeze({ removeIssueStaleWhenUpdated: undefined, removePrStaleWhenUpdated: undefined, ascending: false, - skipStaleIssueMessage: false, - skipStalePrMessage: false, deleteBranch: false, startDate: '', exemptMilestones: '', diff --git a/__tests__/main.spec.ts b/__tests__/main.spec.ts index 3a86eb227..1c56531f7 100644 --- a/__tests__/main.spec.ts +++ b/__tests__/main.spec.ts @@ -1422,11 +1422,11 @@ test('stale issues should not be closed until after the closed number of days (l expect(processor.staleIssues).toHaveLength(1); }); -test('skips stale message on issues when skip-stale-issue-message is set', async () => { +test('skips stale message on issues when stale-issue-message is empty', async () => { const opts = {...DefaultProcessorOptions}; opts.daysBeforeStale = 5; // stale after 5 days opts.daysBeforeClose = 20; // closes after 25 days - opts.skipStaleIssueMessage = true; + opts.staleIssueMessage = ''; const lastUpdate = new Date(); lastUpdate.setDate(lastUpdate.getDate() - 10); const TestIssueList: Issue[] = [ @@ -1467,11 +1467,11 @@ test('skips stale message on issues when skip-stale-issue-message is set', async ); }); -test('skips stale message on prs when skip-stale-pr-message is set', async () => { +test('send stale message on issues when stale-issue-message is not empty', async () => { const opts = {...DefaultProcessorOptions}; opts.daysBeforeStale = 5; // stale after 5 days opts.daysBeforeClose = 20; // closes after 25 days - opts.skipStalePrMessage = true; + opts.staleIssueMessage = 'dummy issue message'; const lastUpdate = new Date(); lastUpdate.setDate(lastUpdate.getDate() - 10); const TestIssueList: Issue[] = [ @@ -1481,7 +1481,7 @@ test('skips stale message on prs when skip-stale-pr-message is set', async () => 'An issue that should be marked stale but not closed', lastUpdate.toString(), lastUpdate.toString(), - true + false ) ]; const processor = new IssuesProcessorMock( @@ -1505,19 +1505,18 @@ test('skips stale message on prs when skip-stale-pr-message is set', async () => // comment should not be created expect(markSpy).toHaveBeenCalledWith( TestIssueList[0], - opts.stalePrMessage, - opts.stalePrLabel, + opts.staleIssueMessage, + opts.staleIssueLabel, // this option is skipMessage - true + false ); }); -test('not providing state takes precedence over skipStaleIssueMessage', async () => { +test('skips stale message on prs when stale-pr-message is empty', async () => { const opts = {...DefaultProcessorOptions}; opts.daysBeforeStale = 5; // stale after 5 days opts.daysBeforeClose = 20; // closes after 25 days - opts.skipStalePrMessage = true; - opts.staleIssueMessage = ''; + opts.stalePrMessage = ''; const lastUpdate = new Date(); lastUpdate.setDate(lastUpdate.getDate() - 10); const TestIssueList: Issue[] = [ @@ -1527,7 +1526,7 @@ test('not providing state takes precedence over skipStaleIssueMessage', async () 'An issue that should be marked stale but not closed', lastUpdate.toString(), lastUpdate.toString(), - false + true ) ]; const processor = new IssuesProcessorMock( @@ -1538,20 +1537,31 @@ test('not providing state takes precedence over skipStaleIssueMessage', async () async () => new Date().toDateString() ); + // for sake of testing, mocking private function + const markSpy = jest.spyOn(processor as any, '_markStale'); + await processor.processIssues(1); // issue should be staled expect(processor.closedIssues).toHaveLength(0); expect(processor.removedLabelIssues).toHaveLength(0); - expect(processor.staleIssues).toHaveLength(0); + expect(processor.staleIssues).toHaveLength(1); + + // comment should not be created + expect(markSpy).toHaveBeenCalledWith( + TestIssueList[0], + opts.stalePrMessage, + opts.stalePrLabel, + // this option is skipMessage + true + ); }); -test('not providing stalePrMessage takes precedence over skipStalePrMessage', async () => { +test('send stale message on prs when stale-pr-message is not empty', async () => { const opts = {...DefaultProcessorOptions}; opts.daysBeforeStale = 5; // stale after 5 days opts.daysBeforeClose = 20; // closes after 25 days - opts.skipStalePrMessage = true; - opts.stalePrMessage = ''; + opts.stalePrMessage = 'dummy pr message'; const lastUpdate = new Date(); lastUpdate.setDate(lastUpdate.getDate() - 10); const TestIssueList: Issue[] = [ @@ -1572,12 +1582,24 @@ test('not providing stalePrMessage takes precedence over skipStalePrMessage', as async () => new Date().toDateString() ); + // for sake of testing, mocking private function + const markSpy = jest.spyOn(processor as any, '_markStale'); + await processor.processIssues(1); // issue should be staled expect(processor.closedIssues).toHaveLength(0); expect(processor.removedLabelIssues).toHaveLength(0); - expect(processor.staleIssues).toHaveLength(0); + expect(processor.staleIssues).toHaveLength(1); + + // comment should not be created + expect(markSpy).toHaveBeenCalledWith( + TestIssueList[0], + opts.stalePrMessage, + opts.stalePrLabel, + // this option is skipMessage + false + ); }); test('git branch is deleted when option is enabled', async () => { diff --git a/src/classes/issue.spec.ts b/src/classes/issue.spec.ts index 139edf66a..1fd8690f4 100644 --- a/src/classes/issue.spec.ts +++ b/src/classes/issue.spec.ts @@ -38,8 +38,6 @@ describe('Issue', (): void => { removeIssueStaleWhenUpdated: undefined, removePrStaleWhenUpdated: undefined, repoToken: '', - skipStaleIssueMessage: false, - skipStalePrMessage: false, staleIssueMessage: '', stalePrMessage: '', startDate: undefined, diff --git a/src/classes/issues-processor.ts b/src/classes/issues-processor.ts index fac5778fd..6c951bab4 100644 --- a/src/classes/issues-processor.ts +++ b/src/classes/issues-processor.ts @@ -138,8 +138,8 @@ export class IssuesProcessor { ? this.options.closePrLabel : this.options.closeIssueLabel; const skipMessage = issue.isPullRequest - ? this.options.skipStalePrMessage - : this.options.skipStaleIssueMessage; + ? this.options.stalePrMessage.length === 0 + : this.options.staleIssueMessage.length === 0; const daysBeforeStale: number = issue.isPullRequest ? this._getDaysBeforePrStale() : this._getDaysBeforeIssueStale(); @@ -196,20 +196,6 @@ export class IssuesProcessor { const shouldMarkAsStale: boolean = shouldMarkWhenStale(daysBeforeStale); - if (!staleMessage && shouldMarkAsStale) { - issueLogger.info( - `Skipping this $$type because it should be marked as stale based on the option ${issueLogger.createOptionLink( - this._getDaysBeforeStaleUsedOptionName(issue) - )} (${chalk.cyan( - daysBeforeStale - )}) but the option ${issueLogger.createOptionLink( - IssuesProcessor._getStaleMessageUsedOptionName(issue) - )} is not set` - ); - IssuesProcessor._endIssueProcessing(issue); - continue; - } - if (issue.state === 'closed') { issueLogger.info(`Skipping this $$type because it is closed`); IssuesProcessor._endIssueProcessing(issue); diff --git a/src/enums/option.ts b/src/enums/option.ts index 4ce789eea..e588667fd 100644 --- a/src/enums/option.ts +++ b/src/enums/option.ts @@ -24,8 +24,6 @@ export enum Option { RemoveStaleWhenUpdated = 'remove-stale-when-updated', DebugOnly = 'debug-only', Ascending = 'ascending', - SkipStaleIssueMessage = 'skip-stale-issue-message', - SkipStalePrMessage = 'skip-stale-pr-message', DeleteBranch = 'delete-branch', StartDate = 'start-date', ExemptMilestones = 'exempt-milestones', diff --git a/src/interfaces/issues-processor-options.ts b/src/interfaces/issues-processor-options.ts index dc00421d1..6ce79db18 100644 --- a/src/interfaces/issues-processor-options.ts +++ b/src/interfaces/issues-processor-options.ts @@ -30,8 +30,6 @@ export interface IIssuesProcessorOptions { removePrStaleWhenUpdated: boolean | undefined; debugOnly: boolean; ascending: boolean; - skipStaleIssueMessage: boolean; - skipStalePrMessage: boolean; deleteBranch: boolean; startDate: IsoOrRfcDateString | undefined; // Should be ISO 8601 or RFC 2822 exemptMilestones: string; diff --git a/src/main.ts b/src/main.ts index 8a2ff12da..679c5844f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -57,8 +57,6 @@ function _getAndValidateArgs(): IIssuesProcessorOptions { ), debugOnly: core.getInput('debug-only') === 'true', ascending: core.getInput('ascending') === 'true', - skipStalePrMessage: core.getInput('skip-stale-pr-message') === 'true', - skipStaleIssueMessage: core.getInput('skip-stale-issue-message') === 'true', deleteBranch: core.getInput('delete-branch') === 'true', startDate: core.getInput('start-date') !== ''