Remove the stale label when labeled with an exempt one (#268)

* refactor(issue): create a dedicated function to remove the stale label

* refactor: prefix private methods with _

to make it consistent with others methods

* feat(label): remove the stale label when labeled with an exempt one

closes #136

* chore: fix logger issues due to rebase

@hross I think there is a room for improvement regarding the class creation of the issue logger (code duplication) but I do not see how to do it without changing a lot of stuff; do you have an idea?

* test: use strict equal and move the new test in a more logical position

* docs(readme): fix parsing error of the default values in the table

prettier was not liking the previous syntax
This commit is contained in:
Geoffrey Testelin 2021-01-17 02:13:19 +01:00 committed by GitHub
parent 3f95874437
commit 7f340a46f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 112 additions and 58 deletions

View File

@ -27,29 +27,29 @@ $ npm test
| Input | Description | Usage | | Input | Description | Usage |
| --------------------------- | ------------------------------------------------------------------------------------ | -------- | | --------------------------- | ------------------------------------------------------------------------------------ | -------- |
| `repo-token` | PAT(Personal Access Token) for authorizing repository. | Optional | | `repo-token` | PAT(Personal Access Token) for authorizing repository. | Optional |
| `days-before-stale` | Idle number of days before marking an issue/pr as stale. \*Defaults to **60** | Optional | | `days-before-stale` | Idle number of days before marking an issue/pr as stale. _Defaults to **60**_ | Optional |
| `days-before-issue-stale` | Idle number of days before marking an issue as stale (override `days-before-stale`). | Optional | | `days-before-issue-stale` | Idle number of days before marking an issue as stale (override `days-before-stale`). | Optional |
| `days-before-pr-stale` | Idle number of days before marking an pr as stale (override `days-before-stale`). | Optional | | `days-before-pr-stale` | Idle number of days before marking an pr as stale (override `days-before-stale`). | Optional |
| `days-before-close` | Idle number of days before closing an stale issue/pr. \*Defaults to **7\*** | Optional | | `days-before-close` | Idle number of days before closing an stale issue/pr. _Defaults to **7**_ | Optional |
| `days-before-issue-close` | Idle number of days before closing an stale issue (override `days-before-close`). | Optional | | `days-before-issue-close` | Idle number of days before closing an stale issue (override `days-before-close`). | Optional |
| `days-before-pr-close` | Idle number of days before closing an stale pr (override `days-before-close`). | Optional | | `days-before-pr-close` | Idle number of days before closing an stale pr (override `days-before-close`). | Optional |
| `stale-issue-message` | Message to post on the stale issue. | Optional | | `stale-issue-message` | Message to post on the stale issue. | Optional |
| `stale-pr-message` | Message to post on the stale pr. | Optional | | `stale-pr-message` | Message to post on the stale pr. | Optional |
| `close-issue-message` | Message to post on the stale issue while closing it. | Optional | | `close-issue-message` | Message to post on the stale issue while closing it. | Optional |
| `close-pr-message` | Message to post on the stale pr while closing it. | Optional | | `close-pr-message` | Message to post on the stale pr while closing it. | Optional |
| `stale-issue-label` | Label to apply on the stale issue. \*Defaults to **stale\*** | Optional | | `stale-issue-label` | Label to apply on the stale issue. _Defaults to **stale**_ | Optional |
| `close-issue-label` | Label to apply on closing issue. | Optional | | `close-issue-label` | Label to apply on closing issue. | Optional |
| `stale-pr-label` | Label to apply on the stale pr. | Optional | | `stale-pr-label` | Label to apply on the stale pr. | Optional |
| `close-pr-label` | Label to apply on the closing pr. | Optional | | `close-pr-label` | Label to apply on the closing pr. | Optional |
| `exempt-issue-labels` | Labels on an issue exempted from being marked as stale. | Optional | | `exempt-issue-labels` | Labels on an issue exempted from being marked as stale. | Optional |
| `exempt-pr-labels` | Labels on the pr exempted from being marked as stale. | Optional | | `exempt-pr-labels` | Labels on the pr exempted from being marked as stale. | Optional |
| `only-labels` | Only labels checked for stale issue/pr. | Optional | | `only-labels` | Only labels checked for stale issue/pr. | Optional |
| `operations-per-run` | Maximum number of operations per run. \*Defaults to **30\*** | Optional | | `operations-per-run` | Maximum number of operations per run. _Defaults to **30**_ | Optional |
| `remove-stale-when-updated` | Remove stale label from issue/pr on updates or comments. \*Defaults to **true\*** | Optional | | `remove-stale-when-updated` | Remove stale label from issue/pr on updates or comments. _Defaults to **true**_ | Optional |
| `debug-only` | Dry-run on action. \*Defaults to **false\*** | Optional | | `debug-only` | Dry-run on action. _Defaults to **false**_ | Optional |
| `ascending` | Order to get issues/pr. \*Defaults to **false\*** | Optional | | `ascending` | Order to get issues/pr. _Defaults to **false**_ | Optional |
| `skip-stale-issue-message` | Skip adding stale message on stale issue. \*Defaults to **false\*** | Optional | | `skip-stale-issue-message` | Skip adding stale message on stale issue. _Defaults to **false**_ | Optional |
| `skip-stale-pr-message` | Skip adding stale message on stale pr. \*Defaults to **false\*** | Optional | | `skip-stale-pr-message` | Skip adding stale message on stale pr. _Defaults to **false**_ | Optional |
### Usage ### Usage

View File

@ -797,6 +797,7 @@ test('stale locked prs will not be closed', async () => {
}); });
test('exempt issue labels will not be marked stale', async () => { test('exempt issue labels will not be marked stale', async () => {
expect.assertions(3);
const TestIssueList: Issue[] = [ const TestIssueList: Issue[] = [
generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [ generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [
'Exempt' 'Exempt'
@ -817,8 +818,9 @@ test('exempt issue labels will not be marked stale', async () => {
// process our fake issue list // process our fake issue list
await processor.processIssues(1); await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(0); expect(processor.staleIssues.length).toStrictEqual(0);
expect(processor.closedIssues.length).toEqual(0); expect(processor.closedIssues.length).toStrictEqual(0);
expect(processor.removedLabelIssues.length).toStrictEqual(0);
}); });
test('exempt issue labels will not be marked stale (multi issue label with spaces)', async () => { test('exempt issue labels will not be marked stale (multi issue label with spaces)', async () => {
@ -892,6 +894,42 @@ test('exempt pr labels will not be marked stale', async () => {
expect(processor.staleIssues.length).toEqual(2); // PR should get processed even though it has an exempt **issue** label expect(processor.staleIssues.length).toEqual(2); // PR should get processed even though it has an exempt **issue** label
}); });
test('exempt issue labels will not be marked stale and will remove the existing stale label', async () => {
expect.assertions(3);
const TestIssueList: Issue[] = [
generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [
'Exempt',
'Stale'
])
];
const opts = {...DefaultProcessorOptions};
opts.exemptIssueLabels = 'Exempt';
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num: number, dt: string) => [
{
user: {
login: 'notme',
type: 'User'
}
}
], // return a fake comment to indicate there was an update
async (issue: Issue, label: string) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toStrictEqual(0);
expect(processor.closedIssues.length).toStrictEqual(0);
expect(processor.removedLabelIssues.length).toStrictEqual(1);
});
test('stale issues should not be closed if days is set to -1', async () => { test('stale issues should not be closed if days is set to -1', async () => {
const TestIssueList: Issue[] = [ const TestIssueList: Issue[] = [
generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [ generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [
@ -1150,7 +1188,7 @@ test('skips stale message on issues when skip-stale-issue-message is set', async
); );
// for sake of testing, mocking private function // for sake of testing, mocking private function
const markSpy = jest.spyOn(processor as any, 'markStale'); const markSpy = jest.spyOn(processor as any, '_markStale');
await processor.processIssues(1); await processor.processIssues(1);
@ -1195,7 +1233,7 @@ test('skips stale message on prs when skip-stale-pr-message is set', async () =>
); );
// for sake of testing, mocking private function // for sake of testing, mocking private function
const markSpy = jest.spyOn(processor as any, 'markStale'); const markSpy = jest.spyOn(processor as any, '_markStale');
await processor.processIssues(1); await processor.processIssues(1);

View File

@ -80,7 +80,7 @@ const logger: Logger = new Logger();
* Handle processing of issues for staleness/closure. * Handle processing of issues for staleness/closure.
*/ */
export class IssueProcessor { export class IssueProcessor {
private static updatedSince(timestamp: string, num_days: number): boolean { private static _updatedSince(timestamp: string, num_days: number): boolean {
const daysInMillis = 1000 * 60 * 60 * 24 * num_days; const daysInMillis = 1000 * 60 * 60 * 24 * num_days;
const millisSinceLastUpdated = const millisSinceLastUpdated =
new Date().getTime() - new Date(timestamp).getTime(); new Date().getTime() - new Date(timestamp).getTime();
@ -114,19 +114,19 @@ export class IssueProcessor {
this.client = getOctokit(options.repoToken); this.client = getOctokit(options.repoToken);
if (getActor) { if (getActor) {
this.getActor = getActor; this._getActor = getActor;
} }
if (getIssues) { if (getIssues) {
this.getIssues = getIssues; this._getIssues = getIssues;
} }
if (listIssueComments) { if (listIssueComments) {
this.listIssueComments = listIssueComments; this._listIssueComments = listIssueComments;
} }
if (getLabelCreationDate) { if (getLabelCreationDate) {
this.getLabelCreationDate = getLabelCreationDate; this._getLabelCreationDate = getLabelCreationDate;
} }
if (this.options.debugOnly) { if (this.options.debugOnly) {
@ -138,10 +138,10 @@ export class IssueProcessor {
async processIssues(page = 1): Promise<number> { async processIssues(page = 1): Promise<number> {
// get the next batch of issues // get the next batch of issues
const issues: Issue[] = await this.getIssues(page); const issues: Issue[] = await this._getIssues(page);
this.operationsLeft -= 1; this.operationsLeft -= 1;
const actor: string = await this.getActor(); const actor: string = await this._getActor();
if (issues.length <= 0) { if (issues.length <= 0) {
logger.info('---'); logger.info('---');
@ -204,18 +204,7 @@ export class IssueProcessor {
continue; // don't process locked issues continue; // don't process locked issues
} }
if ( // Does this issue have a stale label?
exemptLabels.some((exemptLabel: string) =>
isLabeled(issue, exemptLabel)
)
) {
issueLogger.info(
`Skipping ${issueType} because it has an exempt label`
);
continue; // don't process exempt issues
}
// does this issue have a stale label?
let isStale: boolean = isLabeled(issue, staleLabel); let isStale: boolean = isLabeled(issue, staleLabel);
if (isStale) { if (isStale) {
@ -224,8 +213,24 @@ export class IssueProcessor {
issueLogger.info(`This issue hasn't a stale label`); issueLogger.info(`This issue hasn't a stale label`);
} }
if (
exemptLabels.some((exemptLabel: Readonly<string>): boolean =>
isLabeled(issue, exemptLabel)
)
) {
if (isStale) {
issueLogger.info(`An exempt label was added after the stale label.`);
await this._removeStaleLabel(issue, staleLabel);
}
issueLogger.info(
`Skipping ${issueType} because it has an exempt label`
);
continue; // don't process exempt issues
}
// should this issue be marked stale? // should this issue be marked stale?
const shouldBeStale = !IssueProcessor.updatedSince( const shouldBeStale = !IssueProcessor._updatedSince(
issue.updated_at, issue.updated_at,
this.options.daysBeforeStale this.options.daysBeforeStale
); );
@ -235,14 +240,14 @@ export class IssueProcessor {
issueLogger.info( issueLogger.info(
`Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label` `Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label`
); );
await this.markStale(issue, staleMessage, staleLabel, skipMessage); await this._markStale(issue, staleMessage, staleLabel, skipMessage);
isStale = true; // this issue is now considered stale isStale = true; // this issue is now considered stale
} }
// process the issue if it was marked stale // process the issue if it was marked stale
if (isStale) { if (isStale) {
issueLogger.info(`Found a stale ${issueType}`); issueLogger.info(`Found a stale ${issueType}`);
await this.processStaleIssue( await this._processStaleIssue(
issue, issue,
issueType, issueType,
staleLabel, staleLabel,
@ -263,7 +268,7 @@ export class IssueProcessor {
} }
// handle all of the stale issue logic when we find a stale issue // handle all of the stale issue logic when we find a stale issue
private async processStaleIssue( private async _processStaleIssue(
issue: Issue, issue: Issue,
issueType: IssueType, issueType: IssueType,
staleLabel: string, staleLabel: string,
@ -273,12 +278,12 @@ export class IssueProcessor {
) { ) {
const issueLogger: IssueLogger = new IssueLogger(issue); const issueLogger: IssueLogger = new IssueLogger(issue);
const markedStaleOn: string = const markedStaleOn: string =
(await this.getLabelCreationDate(issue, staleLabel)) || issue.updated_at; (await this._getLabelCreationDate(issue, staleLabel)) || issue.updated_at;
issueLogger.info( issueLogger.info(
`Issue #${issue.number} marked stale on: ${markedStaleOn}` `Issue #${issue.number} marked stale on: ${markedStaleOn}`
); );
const issueHasComments: boolean = await this.hasCommentsSince( const issueHasComments: boolean = await this._hasCommentsSince(
issue, issue,
markedStaleOn, markedStaleOn,
actor actor
@ -298,7 +303,7 @@ export class IssueProcessor {
issueLogger.info(`Days before issue close: ${daysBeforeClose}`); issueLogger.info(`Days before issue close: ${daysBeforeClose}`);
} }
const issueHasUpdate: boolean = IssueProcessor.updatedSince( const issueHasUpdate: boolean = IssueProcessor._updatedSince(
issue.updated_at, issue.updated_at,
daysBeforeClose daysBeforeClose
); );
@ -308,10 +313,7 @@ export class IssueProcessor {
// should we un-stale this issue? // should we un-stale this issue?
if (this.options.removeStaleWhenUpdated && issueHasComments) { if (this.options.removeStaleWhenUpdated && issueHasComments) {
issueLogger.info( await this._removeStaleLabel(issue, staleLabel);
`Issue #${issue.number} is no longer stale. Removing stale label.`
);
await this.removeLabel(issue, staleLabel);
} }
// now start closing logic // now start closing logic
@ -323,13 +325,13 @@ export class IssueProcessor {
issueLogger.info( issueLogger.info(
`Closing ${issueType} because it was last updated on ${issue.updated_at}` `Closing ${issueType} because it was last updated on ${issue.updated_at}`
); );
await this.closeIssue(issue, closeMessage, closeLabel); await this._closeIssue(issue, closeMessage, closeLabel);
if (this.options.deleteBranch && issue.pull_request) { if (this.options.deleteBranch && issue.pull_request) {
issueLogger.info( issueLogger.info(
`Deleting branch for #${issue.number} as delete-branch option was specified` `Deleting branch for #${issue.number} as delete-branch option was specified`
); );
await this.deleteBranch(issue); await this._deleteBranch(issue);
this.deletedBranchIssues.push(issue); this.deletedBranchIssues.push(issue);
} }
} else { } else {
@ -340,7 +342,7 @@ export class IssueProcessor {
} }
// checks to see if a given issue is still stale (has had activity on it) // checks to see if a given issue is still stale (has had activity on it)
private async hasCommentsSince( private async _hasCommentsSince(
issue: Issue, issue: Issue,
sinceDate: string, sinceDate: string,
actor: string actor: string
@ -356,7 +358,7 @@ export class IssueProcessor {
} }
// find any comments since the date // find any comments since the date
const comments = await this.listIssueComments(issue.number, sinceDate); const comments = await this._listIssueComments(issue.number, sinceDate);
const filteredComments = comments.filter( const filteredComments = comments.filter(
comment => comment.user.type === 'User' && comment.user.login !== actor comment => comment.user.type === 'User' && comment.user.login !== actor
@ -371,7 +373,7 @@ export class IssueProcessor {
} }
// grab comments for an issue since a given date // grab comments for an issue since a given date
private async listIssueComments( private async _listIssueComments(
issueNumber: number, issueNumber: number,
sinceDate: string sinceDate: string
): Promise<Comment[]> { ): Promise<Comment[]> {
@ -391,7 +393,7 @@ export class IssueProcessor {
} }
// get the actor from the GitHub token or context // get the actor from the GitHub token or context
private async getActor(): Promise<string> { private async _getActor(): Promise<string> {
let actor; let actor;
try { try {
actor = await this.client.users.getAuthenticated(); actor = await this.client.users.getAuthenticated();
@ -403,7 +405,7 @@ export class IssueProcessor {
} }
// grab issues from github in baches of 100 // grab issues from github in baches of 100
private async getIssues(page: number): Promise<Issue[]> { private async _getIssues(page: number): Promise<Issue[]> {
// generate type for response // generate type for response
const endpoint = this.client.issues.listForRepo; const endpoint = this.client.issues.listForRepo;
type OctoKitIssueList = GetResponseTypeFromEndpointMethod<typeof endpoint>; type OctoKitIssueList = GetResponseTypeFromEndpointMethod<typeof endpoint>;
@ -428,7 +430,7 @@ export class IssueProcessor {
} }
// Mark an issue as stale with a comment and a label // Mark an issue as stale with a comment and a label
private async markStale( private async _markStale(
issue: Issue, issue: Issue,
staleMessage: string, staleMessage: string,
staleLabel: string, staleLabel: string,
@ -477,7 +479,7 @@ export class IssueProcessor {
} }
// Close an issue based on staleness // Close an issue based on staleness
private async closeIssue( private async _closeIssue(
issue: Issue, issue: Issue,
closeMessage?: string, closeMessage?: string,
closeLabel?: string closeLabel?: string
@ -532,9 +534,10 @@ export class IssueProcessor {
} }
} }
private async getPullRequest(issue: Issue): Promise<PullRequest | undefined> { private async _getPullRequest(
issue: Issue
): Promise<PullRequest | undefined> {
const issueLogger: IssueLogger = new IssueLogger(issue); const issueLogger: IssueLogger = new IssueLogger(issue);
this.operationsLeft -= 1; this.operationsLeft -= 1;
try { try {
@ -553,7 +556,7 @@ export class IssueProcessor {
} }
// Delete the branch on closed pull request // Delete the branch on closed pull request
private async deleteBranch(issue: Issue): Promise<void> { private async _deleteBranch(issue: Issue): Promise<void> {
const issueLogger: IssueLogger = new IssueLogger(issue); const issueLogger: IssueLogger = new IssueLogger(issue);
issueLogger.info( issueLogger.info(
@ -564,7 +567,7 @@ export class IssueProcessor {
return; return;
} }
const pullRequest = await this.getPullRequest(issue); const pullRequest = await this._getPullRequest(issue);
if (!pullRequest) { if (!pullRequest) {
issueLogger.info( issueLogger.info(
@ -594,7 +597,7 @@ export class IssueProcessor {
} }
// Remove a label from an issue // Remove a label from an issue
private async removeLabel(issue: Issue, label: string): Promise<void> { private async _removeLabel(issue: Issue, label: string): Promise<void> {
const issueLogger: IssueLogger = new IssueLogger(issue); const issueLogger: IssueLogger = new IssueLogger(issue);
issueLogger.info(`Removing label "${label}" from issue #${issue.number}`); issueLogger.info(`Removing label "${label}" from issue #${issue.number}`);
@ -622,7 +625,7 @@ export class IssueProcessor {
// returns the creation date of a given label on an issue (or nothing if no label existed) // returns the creation date of a given label on an issue (or nothing if no label existed)
///see https://developer.github.com/v3/activity/events/ ///see https://developer.github.com/v3/activity/events/
private async getLabelCreationDate( private async _getLabelCreationDate(
issue: Issue, issue: Issue,
label: string label: string
): Promise<string | undefined> { ): Promise<string | undefined> {
@ -677,4 +680,17 @@ export class IssueProcessor {
? this.options.daysBeforeClose ? this.options.daysBeforeClose
: this.options.daysBeforePrClose; : this.options.daysBeforePrClose;
} }
private async _removeStaleLabel(
issue: Readonly<Issue>,
staleLabel: Readonly<string>
): Promise<void> {
const issueLogger: IssueLogger = new IssueLogger(issue);
issueLogger.info(
`Issue #${issue.number} is no longer stale. Removing stale label.`
);
return this._removeLabel(issue, staleLabel);
}
} }