Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(options): simplify config by removing skip stale message options #457

Merged
merged 3 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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
  • Loading branch information
C0ZEN committed May 25, 2021
commit 73417c5792836e8b5d6fab7972202557151445fd
2 changes: 0 additions & 2 deletions __tests__/constants/default-processor-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ export const DefaultProcessorOptions: IIssuesProcessorOptions = Object.freeze({
removeIssueStaleWhenUpdated: undefined,
removePrStaleWhenUpdated: undefined,
ascending: false,
skipStaleIssueMessage: false,
skipStalePrMessage: false,
deleteBranch: false,
startDate: '',
exemptMilestones: '',
Expand Down
56 changes: 39 additions & 17 deletions __tests__/main.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [
Expand Down Expand Up @@ -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[] = [
Expand All @@ -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(
Expand All @@ -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[] = [
Expand All @@ -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(
Expand All @@ -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[] = [
Expand All @@ -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 () => {
Expand Down
2 changes: 0 additions & 2 deletions src/classes/issue.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ describe('Issue', (): void => {
removeIssueStaleWhenUpdated: undefined,
removePrStaleWhenUpdated: undefined,
repoToken: '',
skipStaleIssueMessage: false,
skipStalePrMessage: false,
staleIssueMessage: '',
stalePrMessage: '',
startDate: undefined,
Expand Down
18 changes: 2 additions & 16 deletions src/classes/issues-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions src/enums/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/issues-processor-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') !== ''
Expand Down