From 2f1eed745822a17d31d5b6f7691aaac158b90df0 Mon Sep 17 00:00:00 2001 From: Mike Penz Date: Fri, 16 Oct 2020 19:30:10 +0200 Subject: [PATCH] - add additional settings to ensure we won't spend the whole API quota if a merge included non anticipated commits --- README.md | 7 +++++++ configuration_complex.json | 6 ++++++ src/configuration.ts | 20 ++++++++++++++------ src/pullRequests.ts | 24 +++++++++++++++++++++--- src/releaseNotes.ts | 28 +++++++++++++++++++--------- src/tags.ts | 14 +++++++------- src/transform.ts | 2 +- 7 files changed, 75 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index d0ee7a5..d59a20c 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,18 @@ By default the action will look for a file called `configuration.json` within th "pattern": "[\\-\\*] (\\[(...|TEST|CI|SKIP)\\])( )?(.+?)\n(.+?[\\-\\*] )(.+)", "target": "- $4\n - $6" } + ], + "max_tags_to_fetch": 200, + "max_pull_requests": 200, + "max_back_track_time_days": 90, + "exclude_merge_branches": [ + "Owner/qa" ] } ``` Any section of the configruation can be ommited, to have defaults apply +Defaults for the configuraiton can be found in the [configuration.ts](https://github.com/mikepenz/release-changelog-builder-action/blob/develop/src/configuration.ts) ## Advanced workflow specification diff --git a/configuration_complex.json b/configuration_complex.json index d000086..9759fb8 100644 --- a/configuration_complex.json +++ b/configuration_complex.json @@ -26,5 +26,11 @@ "pattern": "[\\-\\*] (\\[(...|TEST|CI|SKIP)\\])( )?(.+?)\n(.+?[\\-\\*] )(.+)", "target": "- $4\n - $6" } + ], + "max_tags_to_fetch": 200, + "max_pull_requests": 200, + "max_back_track_time_days": 90, + "exclude_merge_branches": [ + "Owner/qa" ] } \ No newline at end of file diff --git a/src/configuration.ts b/src/configuration.ts index ead0b38..63db05f 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -1,4 +1,8 @@ export interface Configuration { + max_tags_to_fetch: number, + max_pull_requests: number, + max_back_track_time_days: number, + exclude_merge_branches: string[], sort: string template: string pr_template: string @@ -18,10 +22,14 @@ export interface Transformer { } export const DefaultConfiguration: Configuration = { - sort: 'ASC', - template: '${{CHANGELOG}}', - pr_template: '- ${{TITLE}}\n - PR: #${{NUMBER}}', - empty_template: '- no changes', - categories: [], - transformers: [] + max_tags_to_fetch: 200, // the amount of tags to fetch from the github API + max_pull_requests: 200, // the amount of pull requests to process + max_back_track_time_days: 90, // allow max of 90 days to check up on pull requests + exclude_merge_branches: [], // branches to exclude from counting as PRs (e.g. YourOrg/qa, YourOrg/main) + sort: 'ASC', // sorting order for filling the changelog (ASC or DESC) supported + template: '${{CHANGELOG}}', // the global template to host the changelog + pr_template: '- ${{TITLE}}\n - PR: #${{NUMBER}}', // the per PR template to pick + empty_template: '- no changes', // the template to use if no pull requests are found + categories: [], // the categories to support for the ordering + transformers: [] // transformers to apply on the PR description according to the `pr_template` } diff --git a/src/pullRequests.ts b/src/pullRequests.ts index 7d50489..fadb0f0 100755 --- a/src/pullRequests.ts +++ b/src/pullRequests.ts @@ -52,7 +52,8 @@ export class PullRequests { owner: string, repo: string, fromDate: moment.Moment, - toDate: moment.Moment // eslint-disable-line @typescript-eslint/no-unused-vars + toDate: moment.Moment, // eslint-disable-line @typescript-eslint/no-unused-vars + maxPullRequests: number ): Promise { const mergedPRs: PullRequestInfo[] = [] const options = this.octokit.pulls.list.endpoint.merge({ @@ -83,7 +84,11 @@ export class PullRequests { } const firstPR = prs[0] - if (firstPR.merged_at && fromDate.isAfter(moment(firstPR.merged_at))) { + if (firstPR.merged_at && fromDate.isAfter(moment(firstPR.merged_at)) || mergedPRs.length >= maxPullRequests) { + if( mergedPRs.length >= maxPullRequests ) { + core.info(`Reached 'maxPullRequests' count ${maxPullRequests}`) + } + // bail out early to not keep iterating on PRs super old return sortPullRequests(mergedPRs, true) } @@ -92,11 +97,24 @@ export class PullRequests { return sortPullRequests(mergedPRs, true) } - filterCommits(commits: CommitInfo[]): CommitInfo[] { + filterCommits(commits: CommitInfo[], excludeMergeBranches: string[]): CommitInfo[] { const prRegex = /Merge pull request #(\d+)/ const filteredCommits = [] for (const commit of commits) { + if(excludeMergeBranches) { + let matched = false + for (const excludeMergeBranch of excludeMergeBranches) { + if(commit.summary.includes(excludeMergeBranch)) { + matched = true + break + } + } + if(matched) { + continue + } + } + const match = commit.summary.match(prRegex) if (!match) { continue diff --git a/src/releaseNotes.ts b/src/releaseNotes.ts index 04a6593..9dc29e3 100755 --- a/src/releaseNotes.ts +++ b/src/releaseNotes.ts @@ -28,7 +28,7 @@ export class ReleaseNotes { core.debug(`fromTag undefined, trying to resolve via API`) const tagsApi = new Tags(octokit) - const previousTag = await tagsApi.findPredecessorTag(owner, repo, toTag) + const previousTag = await tagsApi.findPredecessorTag(owner, repo, toTag, configuration.max_tags_to_fetch ? configuration.max_tags_to_fetch : DefaultConfiguration.max_tags_to_fetch) if (previousTag == null) { core.error(`Unable to retrieve previous tag given ${toTag}`) return configuration.empty_template @@ -57,7 +57,7 @@ export class ReleaseNotes { private async getMergedPullRequests( octokit: Octokit ): Promise { - const {owner, repo, fromTag, toTag} = this.options + const {owner, repo, fromTag, toTag, configuration} = this.options core.info(`Comparing ${owner}/${repo} - ${fromTag}...${toTag}`) const commitsApi = new Commits(octokit) @@ -69,11 +69,18 @@ export class ReleaseNotes { const firstCommit = commits[0] const lastCommit = commits[commits.length - 1] - const fromDate = firstCommit.date + let fromDate = firstCommit.date const toDate = lastCommit.date + const maxDays = configuration.max_back_track_time_days ? configuration.max_back_track_time_days : DefaultConfiguration.max_back_track_time_days + const maxFromDate = toDate.clone().subtract(maxDays, "days") + if(maxFromDate.isAfter(fromDate)) { + core.info(`Adjusted 'fromDate' to go max ${maxDays} back`) + fromDate = maxFromDate + } + core.info( - `Fetching PRs between dates ${fromDate.toISOString()} ${toDate.toISOString()} for ${owner}/${repo}` + `Fetching PRs between dates ${fromDate.toISOString()} to ${toDate.toISOString()} for ${owner}/${repo}` ) const pullRequestsApi = new PullRequests(octokit) @@ -81,12 +88,16 @@ export class ReleaseNotes { owner, repo, fromDate, - toDate + toDate, + configuration.max_pull_requests ? configuration.max_pull_requests : DefaultConfiguration.max_pull_requests ) - core.info(`Found ${pullRequests.length} merged PRs for ${owner}/${repo}`) + core.info(`Retrieved ${pullRequests.length} merged PRs for ${owner}/${repo}`) + + const prCommits = pullRequestsApi.filterCommits(commits, configuration.exclude_merge_branches ? configuration.exclude_merge_branches : DefaultConfiguration.exclude_merge_branches) + + core.info(`Retrieved ${prCommits.length} PR merge commits for ${owner}/${repo}`) - const prCommits = pullRequestsApi.filterCommits(commits) const filteredPullRequests = [] const pullRequestsByNumber: {[key: number]: PullRequestInfo} = {} @@ -104,7 +115,6 @@ export class ReleaseNotes { if (pullRequestsByNumber[commit.prNumber]) { filteredPullRequests.push(pullRequestsByNumber[commit.prNumber]) } else if (fromDate.toISOString() === toDate.toISOString()) { - core.info(`${prRef} not in date range, fetching explicitly`) const pullRequest = await pullRequestsApi.getSingle( owner, repo, @@ -118,7 +128,7 @@ export class ReleaseNotes { } } else { core.info( - `${prRef} not in date range, likely a merge commit from a fork-to-fork PR` + `${prRef} not in date range, excluding from changelog` ) } } diff --git a/src/tags.ts b/src/tags.ts index afb48de..6ea970c 100755 --- a/src/tags.ts +++ b/src/tags.ts @@ -9,7 +9,7 @@ export interface TagInfo { export class Tags { constructor(private octokit: Octokit) {} - async getTags(owner: string, repo: string): Promise { + async getTags(owner: string, repo: string, maxTagsToFetch: number): Promise { const tagsInfo: TagInfo[] = [] const options = this.octokit.repos.listTags.endpoint.merge({ owner, @@ -18,7 +18,6 @@ export class Tags { per_page: 100 }) - const max = 200 for await (const response of this.octokit.paginate.iterator(options)) { type TagsListData = RestEndpointMethodTypes['repos']['listTags']['response']['data'] const tags: TagsListData = response.data as TagsListData @@ -30,14 +29,14 @@ export class Tags { }) } - // for performance only fetch newest 200 tags!! - if (tagsInfo.length >= max) { + // for performance only fetch newest maxTagsToFetch tags!! + if (tagsInfo.length >= maxTagsToFetch) { break } } core.info( - `Found ${tagsInfo.length} (fetching max: ${max}) tags from the GitHub API for ${owner}/${repo}` + `Found ${tagsInfo.length} (fetching max: ${maxTagsToFetch}) tags from the GitHub API for ${owner}/${repo}` ) return tagsInfo } @@ -45,9 +44,10 @@ export class Tags { async findPredecessorTag( owner: string, repo: string, - tag: string + tag: string, + maxTagsToFetch: number ): Promise { - const tags = this.sortTags(await this.getTags(owner, repo)) + const tags = this.sortTags(await this.getTags(owner, repo, maxTagsToFetch)) const length = tags.length for (let i = 0; i < length; i++) { diff --git a/src/transform.ts b/src/transform.ts index c808750..90566cc 100644 --- a/src/transform.ts +++ b/src/transform.ts @@ -105,7 +105,7 @@ function fillTemplate(pr: PullRequestInfo, template: string): string { transformed = transformed.replace('${{NUMBER}}', pr.number.toString()) transformed = transformed.replace('${{TITLE}}', pr.title) transformed = transformed.replace('${{URL}}', pr.htmlURL) - transformed = transformed.replace('${{MERGED_AT}}', pr.mergedAt.toString()) + transformed = transformed.replace('${{MERGED_AT}}', pr.mergedAt.toISOString()) transformed = transformed.replace('${{AUTHOR}}', pr.author) transformed = transformed.replace('${{BODY}}', pr.body) return transformed