From 77a1abc9e8e7ff69327db6807dd276b70ca9bbad Mon Sep 17 00:00:00 2001 From: Geoffrey Testelin Date: Mon, 7 Jun 2021 15:49:49 +0200 Subject: [PATCH] chore(logs): add more logs and better dry-run (#463) * feat(logs): improve the logs when removing the close label A log was missing when the option was configured but the close label was not present on the issue/PR Change the log about not removing the close label to explicitly mention that this behaviour occur because the option is not configured Fixes #462 * feat(dry-run): display the logs and count the operations the dry-run checks were cancelling way sooner the workflow the logs and the count of operations could not occur in dry run due to this the goal of the dry run is just to skip some logic regarding the api calls to avoid altering the repos but the goal IMO is to also make them reflect the real world run so this change allow this BTW it also allow now to test the consumed operations Fixes #461 * feat(logs): add more logs to debug the stale label removal * chore(index): update index * chore(dry-run): fix bad dry-run conditions --- dist/index.js | 142 +++++++++++++---------- src/classes/issues-processor.ts | 197 ++++++++++++++++++++++---------- src/enums/option.ts | 2 + 3 files changed, 221 insertions(+), 120 deletions(-) diff --git a/dist/index.js b/dist/index.js index 86c6e58d..f228e1f6 100644 --- a/dist/index.js +++ b/dist/index.js @@ -282,6 +282,9 @@ class IssuesProcessor { ? option_1.Option.StalePrMessage : option_1.Option.StaleIssueMessage; } + static _getCloseLabelUsedOptionName(issue) { + return issue.isPullRequest ? option_1.Option.ClosePrLabel : option_1.Option.CloseIssueLabel; + } processIssues(page = 1) { var _a, _b; return __awaiter(this, void 0, void 0, function* () { @@ -557,21 +560,30 @@ class IssuesProcessor { issueLogger.info(`Days before $$type close: ${logger_service_1.LoggerService.cyan(daysBeforeClose)}`); const issueHasUpdate = IssuesProcessor._updatedSince(issue.updated_at, daysBeforeClose); issueLogger.info(`$$type has been updated: ${logger_service_1.LoggerService.cyan(issueHasUpdate)}`); - // should we un-stale this issue? - if (this._shouldRemoveStaleWhenUpdated(issue) && issueHasComments) { + const shouldRemoveStaleWhenUpdated = this._shouldRemoveStaleWhenUpdated(issue); + issueLogger.info(`The option ${issueLogger.createOptionLink(this._getRemoveStaleWhenUpdatedUsedOptionName(issue))} is: ${logger_service_1.LoggerService.cyan(shouldRemoveStaleWhenUpdated)}`); + if (shouldRemoveStaleWhenUpdated) { + issueLogger.info(`The stale label should not be removed`); + } + else { + issueLogger.info(`The stale label should be removed if all conditions met`); + } + // Should we un-stale this issue? + if (shouldRemoveStaleWhenUpdated && issueHasComments) { + issueLogger.info(`Remove the stale label since the $$type has a comment and the workflow should remove the stale label when updated`); yield this._removeStaleLabel(issue, staleLabel); issueLogger.info(`Skipping the process since the $$type is now un-stale`); - return; // nothing to do because it is no longer stale + return; // Nothing to do because it is no longer stale } - // now start closing logic + // Now start closing logic if (daysBeforeClose < 0) { - return; // nothing to do because we aren't closing stale issues + return; // Nothing to do because we aren't closing stale issues } if (!issueHasComments && !issueHasUpdate) { - issueLogger.info(`Closing $$type because it was last updated on! ${logger_service_1.LoggerService.cyan(issue.updated_at)}`); + issueLogger.info(`Closing $$type because it was last updated on: ${logger_service_1.LoggerService.cyan(issue.updated_at)}`); yield this._closeIssue(issue, closeMessage, closeLabel); if (this.options.deleteBranch && issue.pull_request) { - issueLogger.info(`Deleting the branch the option ${issueLogger.createOptionLink(option_1.Option.DeleteBranch)} was specified`); + issueLogger.info(`Deleting the branch since the option ${issueLogger.createOptionLink(option_1.Option.DeleteBranch)} was specified`); yield this._deleteBranch(issue); this.deletedBranchIssues.push(issue); } @@ -608,19 +620,18 @@ class IssuesProcessor { // so that close calculations work correctly const newUpdatedAtDate = new Date(); issue.updated_at = newUpdatedAtDate.toString(); - if (this.options.debugOnly) { - return; - } if (!skipMessage) { try { this._consumeIssueOperation(issue); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementAddedItemsComment(issue); - yield this.client.issues.createComment({ - owner: github_1.context.repo.owner, - repo: github_1.context.repo.repo, - issue_number: issue.number, - body: staleMessage - }); + if (!this.options.debugOnly) { + yield this.client.issues.createComment({ + owner: github_1.context.repo.owner, + repo: github_1.context.repo.repo, + issue_number: issue.number, + body: staleMessage + }); + } } catch (error) { issueLogger.error(`Error when creating a comment: ${error.message}`); @@ -649,19 +660,18 @@ class IssuesProcessor { const issueLogger = new issue_logger_1.IssueLogger(issue); issueLogger.info(`Closing $$type for being stale`); this.closedIssues.push(issue); - if (this.options.debugOnly) { - return; - } if (closeMessage) { try { this._consumeIssueOperation(issue); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementAddedItemsComment(issue); - yield this.client.issues.createComment({ - owner: github_1.context.repo.owner, - repo: github_1.context.repo.repo, - issue_number: issue.number, - body: closeMessage - }); + if (!this.options.debugOnly) { + yield this.client.issues.createComment({ + owner: github_1.context.repo.owner, + repo: github_1.context.repo.repo, + issue_number: issue.number, + body: closeMessage + }); + } } catch (error) { issueLogger.error(`Error when creating a comment: ${error.message}`); @@ -701,18 +711,17 @@ class IssuesProcessor { var _a; return __awaiter(this, void 0, void 0, function* () { const issueLogger = new issue_logger_1.IssueLogger(issue); - if (this.options.debugOnly) { - return; - } try { this._consumeIssueOperation(issue); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementFetchedPullRequestsCount(); - const pullRequest = yield this.client.pulls.get({ - owner: github_1.context.repo.owner, - repo: github_1.context.repo.repo, - pull_number: issue.number - }); - return pullRequest.data; + if (!this.options.debugOnly) { + const pullRequest = yield this.client.pulls.get({ + owner: github_1.context.repo.owner, + repo: github_1.context.repo.repo, + pull_number: issue.number + }); + return pullRequest.data; + } } catch (error) { issueLogger.error(`Error when getting this $$type: ${error.message}`); @@ -730,19 +739,18 @@ class IssuesProcessor { issueLogger.info(`Not deleting this branch as no pull request was found for this $$type`); return; } - if (this.options.debugOnly) { - return; - } const branch = pullRequest.head.ref; issueLogger.info(`Deleting the branch "${logger_service_1.LoggerService.cyan(branch)}" from closed $$type`); try { this._consumeIssueOperation(issue); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementDeletedBranchesCount(); - yield this.client.git.deleteRef({ - owner: github_1.context.repo.owner, - repo: github_1.context.repo.repo, - ref: `heads/${branch}` - }); + if (!this.options.debugOnly) { + yield this.client.git.deleteRef({ + owner: github_1.context.repo.owner, + repo: github_1.context.repo.repo, + ref: `heads/${branch}` + }); + } } catch (error) { issueLogger.error(`Error when deleting the branch "${logger_service_1.LoggerService.cyan(branch)}" from $$type: ${error.message}`); @@ -750,28 +758,27 @@ class IssuesProcessor { }); } // Remove a label from an issue or a pull request - _removeLabel(issue, label) { + _removeLabel(issue, label, isSubStep = false) { var _a; return __awaiter(this, void 0, void 0, function* () { const issueLogger = new issue_logger_1.IssueLogger(issue); - issueLogger.info(`Removing the label "${logger_service_1.LoggerService.cyan(label)}" from this $$type...`); + issueLogger.info(`${isSubStep ? logger_service_1.LoggerService.white('├── ') : ''}Removing the label "${logger_service_1.LoggerService.cyan(label)}" from this $$type...`); this.removedLabelIssues.push(issue); - if (this.options.debugOnly) { - return; - } try { this._consumeIssueOperation(issue); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementDeletedItemsLabelsCount(issue); - yield this.client.issues.removeLabel({ - owner: github_1.context.repo.owner, - repo: github_1.context.repo.repo, - issue_number: issue.number, - name: label - }); - issueLogger.info(`The label "${logger_service_1.LoggerService.cyan(label)}" was removed`); + if (!this.options.debugOnly) { + yield this.client.issues.removeLabel({ + owner: github_1.context.repo.owner, + repo: github_1.context.repo.repo, + issue_number: issue.number, + name: label + }); + } + issueLogger.info(`${isSubStep ? logger_service_1.LoggerService.white('└── ') : ''}The label "${logger_service_1.LoggerService.cyan(label)}" was removed`); } catch (error) { - issueLogger.error(`Error when removing the label: "${logger_service_1.LoggerService.cyan(error.message)}"`); + issueLogger.error(`${isSubStep ? logger_service_1.LoggerService.white('└── ') : ''}Error when removing the label: "${logger_service_1.LoggerService.cyan(error.message)}"`); } }); } @@ -848,14 +855,19 @@ class IssuesProcessor { const issueLogger = new issue_logger_1.IssueLogger(issue); issueLogger.info(`The $$type is not closed nor locked. Trying to remove the close label...`); if (!closeLabel) { - issueLogger.info(`There is no close label on this $$type. Skip`); + issueLogger.info(logger_service_1.LoggerService.white('├──'), `The ${issueLogger.createOptionLink(IssuesProcessor._getCloseLabelUsedOptionName(issue))} option was not set`); + issueLogger.info(logger_service_1.LoggerService.white('└──'), `Skipping the removal of the close label`); return Promise.resolve(); } if (is_labeled_1.isLabeled(issue, closeLabel)) { - issueLogger.info(`The $$type has a close label "${logger_service_1.LoggerService.cyan(closeLabel)}". Removing the close label...`); - yield this._removeLabel(issue, closeLabel); + issueLogger.info(logger_service_1.LoggerService.white('├──'), `The $$type has a close label "${logger_service_1.LoggerService.cyan(closeLabel)}". Removing the close label...`); + yield this._removeLabel(issue, closeLabel, true); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementDeletedCloseItemsLabelsCount(issue); } + else { + issueLogger.info(logger_service_1.LoggerService.white('└──'), `There is no close label on this $$type. Skipping`); + return Promise.resolve(); + } }); } _consumeIssueOperation(issue) { @@ -877,6 +889,18 @@ class IssuesProcessor { ? option_1.Option.DaysBeforeStale : option_1.Option.DaysBeforePrStale; } + _getRemoveStaleWhenUpdatedUsedOptionName(issue) { + if (issue.isPullRequest) { + if (is_boolean_1.isBoolean(this.options.removePrStaleWhenUpdated)) { + return option_1.Option.RemovePrStaleWhenUpdated; + } + return option_1.Option.RemoveStaleWhenUpdated; + } + if (is_boolean_1.isBoolean(this.options.removeIssueStaleWhenUpdated)) { + return option_1.Option.RemoveIssueStaleWhenUpdated; + } + return option_1.Option.RemoveStaleWhenUpdated; + } } exports.IssuesProcessor = IssuesProcessor; @@ -1604,6 +1628,8 @@ var Option; Option["AnyOfLabels"] = "any-of-labels"; Option["OperationsPerRun"] = "operations-per-run"; Option["RemoveStaleWhenUpdated"] = "remove-stale-when-updated"; + Option["RemoveIssueStaleWhenUpdated"] = "remove-issue-stale-when-updated"; + Option["RemovePrStaleWhenUpdated"] = "remove-pr-stale-when-updated"; Option["DebugOnly"] = "debug-only"; Option["Ascending"] = "ascending"; Option["DeleteBranch"] = "delete-branch"; diff --git a/src/classes/issues-processor.ts b/src/classes/issues-processor.ts index 0706ec7d..72f5fcbd 100644 --- a/src/classes/issues-processor.ts +++ b/src/classes/issues-processor.ts @@ -59,6 +59,12 @@ export class IssuesProcessor { : Option.StaleIssueMessage; } + private static _getCloseLabelUsedOptionName( + issue: Readonly + ): Option.ClosePrLabel | Option.CloseIssueLabel { + return issue.isPullRequest ? Option.ClosePrLabel : Option.CloseIssueLabel; + } + private readonly _logger: Logger = new Logger(); private readonly _operations: StaleOperations; private readonly _statistics: Statistics | undefined; @@ -565,23 +571,44 @@ export class IssuesProcessor { `$$type has been updated: ${LoggerService.cyan(issueHasUpdate)}` ); - // should we un-stale this issue? - if (this._shouldRemoveStaleWhenUpdated(issue) && issueHasComments) { + const shouldRemoveStaleWhenUpdated: boolean = this._shouldRemoveStaleWhenUpdated( + issue + ); + + issueLogger.info( + `The option ${issueLogger.createOptionLink( + this._getRemoveStaleWhenUpdatedUsedOptionName(issue) + )} is: ${LoggerService.cyan(shouldRemoveStaleWhenUpdated)}` + ); + + if (shouldRemoveStaleWhenUpdated) { + issueLogger.info(`The stale label should not be removed`); + } else { + issueLogger.info( + `The stale label should be removed if all conditions met` + ); + } + + // Should we un-stale this issue? + if (shouldRemoveStaleWhenUpdated && issueHasComments) { + issueLogger.info( + `Remove the stale label since the $$type has a comment and the workflow should remove the stale label when updated` + ); await this._removeStaleLabel(issue, staleLabel); issueLogger.info(`Skipping the process since the $$type is now un-stale`); - return; // nothing to do because it is no longer stale + return; // Nothing to do because it is no longer stale } - // now start closing logic + // Now start closing logic if (daysBeforeClose < 0) { - return; // nothing to do because we aren't closing stale issues + return; // Nothing to do because we aren't closing stale issues } if (!issueHasComments && !issueHasUpdate) { issueLogger.info( - `Closing $$type because it was last updated on! ${LoggerService.cyan( + `Closing $$type because it was last updated on: ${LoggerService.cyan( issue.updated_at )}` ); @@ -589,7 +616,7 @@ export class IssuesProcessor { if (this.options.deleteBranch && issue.pull_request) { issueLogger.info( - `Deleting the branch the option ${issueLogger.createOptionLink( + `Deleting the branch since the option ${issueLogger.createOptionLink( Option.DeleteBranch )} was specified` ); @@ -653,20 +680,19 @@ export class IssuesProcessor { const newUpdatedAtDate: Date = new Date(); issue.updated_at = newUpdatedAtDate.toString(); - if (this.options.debugOnly) { - return; - } - if (!skipMessage) { try { this._consumeIssueOperation(issue); this._statistics?.incrementAddedItemsComment(issue); - await this.client.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issue.number, - body: staleMessage - }); + + if (!this.options.debugOnly) { + await this.client.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: staleMessage + }); + } } catch (error) { issueLogger.error(`Error when creating a comment: ${error.message}`); } @@ -698,20 +724,19 @@ export class IssuesProcessor { issueLogger.info(`Closing $$type for being stale`); this.closedIssues.push(issue); - if (this.options.debugOnly) { - return; - } - if (closeMessage) { try { this._consumeIssueOperation(issue); this._statistics?.incrementAddedItemsComment(issue); - await this.client.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issue.number, - body: closeMessage - }); + + if (!this.options.debugOnly) { + await this.client.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: closeMessage + }); + } } catch (error) { issueLogger.error(`Error when creating a comment: ${error.message}`); } @@ -751,20 +776,19 @@ export class IssuesProcessor { ): Promise { const issueLogger: IssueLogger = new IssueLogger(issue); - if (this.options.debugOnly) { - return; - } - try { this._consumeIssueOperation(issue); this._statistics?.incrementFetchedPullRequestsCount(); - const pullRequest = await this.client.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: issue.number - }); - return pullRequest.data; + if (!this.options.debugOnly) { + const pullRequest = await this.client.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: issue.number + }); + + return pullRequest.data; + } } catch (error) { issueLogger.error(`Error when getting this $$type: ${error.message}`); } @@ -785,10 +809,6 @@ export class IssuesProcessor { return; } - if (this.options.debugOnly) { - return; - } - const branch = pullRequest.head.ref; issueLogger.info( `Deleting the branch "${LoggerService.cyan(branch)}" from closed $$type` @@ -797,11 +817,14 @@ export class IssuesProcessor { try { this._consumeIssueOperation(issue); this._statistics?.incrementDeletedBranchesCount(); - await this.client.git.deleteRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: `heads/${branch}` - }); + + if (!this.options.debugOnly) { + await this.client.git.deleteRef({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: `heads/${branch}` + }); + } } catch (error) { issueLogger.error( `Error when deleting the branch "${LoggerService.cyan( @@ -812,31 +835,43 @@ export class IssuesProcessor { } // Remove a label from an issue or a pull request - private async _removeLabel(issue: Issue, label: string): Promise { + private async _removeLabel( + issue: Issue, + label: string, + isSubStep: Readonly = false + ): Promise { const issueLogger: IssueLogger = new IssueLogger(issue); issueLogger.info( - `Removing the label "${LoggerService.cyan(label)}" from this $$type...` + `${ + isSubStep ? LoggerService.white('├── ') : '' + }Removing the label "${LoggerService.cyan(label)}" from this $$type...` ); this.removedLabelIssues.push(issue); - if (this.options.debugOnly) { - return; - } - try { this._consumeIssueOperation(issue); this._statistics?.incrementDeletedItemsLabelsCount(issue); - await this.client.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issue.number, - name: label - }); - issueLogger.info(`The label "${LoggerService.cyan(label)}" was removed`); + + if (!this.options.debugOnly) { + await this.client.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + name: label + }); + } + + issueLogger.info( + `${ + isSubStep ? LoggerService.white('└── ') : '' + }The label "${LoggerService.cyan(label)}" was removed` + ); } catch (error) { issueLogger.error( - `Error when removing the label: "${LoggerService.cyan(error.message)}"` + `${ + isSubStep ? LoggerService.white('└── ') : '' + }Error when removing the label: "${LoggerService.cyan(error.message)}"` ); } } @@ -934,20 +969,37 @@ export class IssuesProcessor { ); if (!closeLabel) { - issueLogger.info(`There is no close label on this $$type. Skip`); + issueLogger.info( + LoggerService.white('├──'), + `The ${issueLogger.createOptionLink( + IssuesProcessor._getCloseLabelUsedOptionName(issue) + )} option was not set` + ); + issueLogger.info( + LoggerService.white('└──'), + `Skipping the removal of the close label` + ); return Promise.resolve(); } if (isLabeled(issue, closeLabel)) { issueLogger.info( + LoggerService.white('├──'), `The $$type has a close label "${LoggerService.cyan( closeLabel )}". Removing the close label...` ); - await this._removeLabel(issue, closeLabel); + await this._removeLabel(issue, closeLabel, true); this._statistics?.incrementDeletedCloseItemsLabelsCount(issue); + } else { + issueLogger.info( + LoggerService.white('└──'), + `There is no close label on this $$type. Skipping` + ); + + return Promise.resolve(); } } @@ -982,4 +1034,25 @@ export class IssuesProcessor { ? Option.DaysBeforeStale : Option.DaysBeforePrStale; } + + private _getRemoveStaleWhenUpdatedUsedOptionName( + issue: Readonly + ): + | Option.RemovePrStaleWhenUpdated + | Option.RemoveStaleWhenUpdated + | Option.RemoveIssueStaleWhenUpdated { + if (issue.isPullRequest) { + if (isBoolean(this.options.removePrStaleWhenUpdated)) { + return Option.RemovePrStaleWhenUpdated; + } + + return Option.RemoveStaleWhenUpdated; + } + + if (isBoolean(this.options.removeIssueStaleWhenUpdated)) { + return Option.RemoveIssueStaleWhenUpdated; + } + + return Option.RemoveStaleWhenUpdated; + } } diff --git a/src/enums/option.ts b/src/enums/option.ts index e588667f..0ff794a0 100644 --- a/src/enums/option.ts +++ b/src/enums/option.ts @@ -22,6 +22,8 @@ export enum Option { AnyOfLabels = 'any-of-labels', OperationsPerRun = 'operations-per-run', RemoveStaleWhenUpdated = 'remove-stale-when-updated', + RemoveIssueStaleWhenUpdated = 'remove-issue-stale-when-updated', + RemovePrStaleWhenUpdated = 'remove-pr-stale-when-updated', DebugOnly = 'debug-only', Ascending = 'ascending', DeleteBranch = 'delete-branch',