- introduce new getForCommitHash to retrieve PRs based on hashes
- reorganize pull request filtering -> if we get PRs from commit hash, open PRs are already included, neither filtering is needed - significantly simplify logic for results from commit hashes (only deduping needed) + filter if we don't want open PRs
This commit is contained in:
@@ -14,6 +14,7 @@ export interface Options {
|
|||||||
toTag: TagInfo // the tag/ref up to
|
toTag: TagInfo // the tag/ref up to
|
||||||
includeOpen: boolean // defines if we should also fetch open pull requests
|
includeOpen: boolean // defines if we should also fetch open pull requests
|
||||||
failOnError: boolean // defines if we should fail the action in case of an error
|
failOnError: boolean // defines if we should fail the action in case of an error
|
||||||
|
fetchViaCommits: boolean // defines if PRs are fetched via the commits identified. This will do 1 API request per commit -> Best for scenarios with squash merges | Or shorter from-to diffs (< 10 commits) | Also effective for shorters diffs for very old PRs
|
||||||
fetchReviewers: boolean // defines if the action should fetch the reviewers for PRs - approved reviewers are not included in the default PR listing
|
fetchReviewers: boolean // defines if the action should fetch the reviewers for PRs - approved reviewers are not included in the default PR listing
|
||||||
fetchReleaseInformation: boolean // defines if the action should fetch the release information for the from and to tag - e.g. the creation date for the associated release
|
fetchReleaseInformation: boolean // defines if the action should fetch the release information for the from and to tag - e.g. the creation date for the associated release
|
||||||
fetchReviews: boolean // defines if the action should fetch the reviews for the PR.
|
fetchReviews: boolean // defines if the action should fetch the reviews for the PR.
|
||||||
@@ -40,6 +41,7 @@ export class PullRequestCollector {
|
|||||||
private includeOpen: boolean = false,
|
private includeOpen: boolean = false,
|
||||||
private failOnError: boolean,
|
private failOnError: boolean,
|
||||||
private ignorePreReleases: boolean,
|
private ignorePreReleases: boolean,
|
||||||
|
private fetchViaCommits: boolean = false,
|
||||||
private fetchReviewers: boolean = false,
|
private fetchReviewers: boolean = false,
|
||||||
private fetchReleaseInformation: boolean = false,
|
private fetchReleaseInformation: boolean = false,
|
||||||
private fetchReviews: boolean = false,
|
private fetchReviews: boolean = false,
|
||||||
@@ -119,6 +121,7 @@ export class PullRequestCollector {
|
|||||||
toTag: thisTag,
|
toTag: thisTag,
|
||||||
includeOpen: this.includeOpen,
|
includeOpen: this.includeOpen,
|
||||||
failOnError: this.failOnError,
|
failOnError: this.failOnError,
|
||||||
|
fetchViaCommits: this.fetchViaCommits,
|
||||||
fetchReviewers: this.fetchReviewers,
|
fetchReviewers: this.fetchReviewers,
|
||||||
fetchReleaseInformation: this.fetchReleaseInformation,
|
fetchReleaseInformation: this.fetchReleaseInformation,
|
||||||
fetchReviews: this.fetchReviews,
|
fetchReviews: this.fetchReviews,
|
||||||
|
|||||||
@@ -91,6 +91,28 @@ export class PullRequests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async getForCommitHash(owner: string, repo: string, commit_sha: string, maxPullRequests: number): Promise<PullRequestInfo[]> {
|
||||||
|
const mergedPRs: PullRequestInfo[] = []
|
||||||
|
|
||||||
|
const options = this.octokit.repos.listPullRequestsAssociatedWithCommit.endpoint.merge({
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
commit_sha,
|
||||||
|
per_page: `${Math.min(10, maxPullRequests)}`,
|
||||||
|
direction: 'desc'
|
||||||
|
})
|
||||||
|
|
||||||
|
for await (const response of this.octokit.paginate.iterator(options)) {
|
||||||
|
const prs: PullsListData = response.data as PullsListData
|
||||||
|
|
||||||
|
for (const pr of prs) {
|
||||||
|
mergedPRs.push(mapPullRequest(pr, !!pr.merged_at ? 'merged' : 'open'))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return sortPrs(mergedPRs)
|
||||||
|
}
|
||||||
|
|
||||||
async getBetweenDates(
|
async getBetweenDates(
|
||||||
owner: string,
|
owner: string,
|
||||||
repo: string,
|
repo: string,
|
||||||
@@ -103,7 +125,7 @@ export class PullRequests {
|
|||||||
owner,
|
owner,
|
||||||
repo,
|
repo,
|
||||||
state: 'closed',
|
state: 'closed',
|
||||||
sort: 'updated',
|
sort: 'merged',
|
||||||
per_page: `${Math.min(100, maxPullRequests)}`,
|
per_page: `${Math.min(100, maxPullRequests)}`,
|
||||||
direction: 'desc'
|
direction: 'desc'
|
||||||
})
|
})
|
||||||
@@ -126,7 +148,7 @@ export class PullRequests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// bail out early to not keep iterating on PRs super old
|
// bail out early to not keep iterating on PRs super old
|
||||||
return sortPrs(mergedPRs)
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -158,7 +180,7 @@ export class PullRequests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// bail out early to not keep iterating on PRs super old
|
// bail out early to not keep iterating on PRs super old
|
||||||
return sortPrs(openPrs)
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -207,21 +229,37 @@ export class PullRequests {
|
|||||||
|
|
||||||
core.info(`ℹ️ Fetching PRs between dates ${fromDate.toISOString()} to ${toDate.toISOString()} for ${owner}/${repo}`)
|
core.info(`ℹ️ Fetching PRs between dates ${fromDate.toISOString()} to ${toDate.toISOString()} for ${owner}/${repo}`)
|
||||||
|
|
||||||
const pullRequests = await this.getBetweenDates(owner, repo, fromDate, toDate, configuration.max_pull_requests)
|
|
||||||
|
|
||||||
core.info(`ℹ️ Retrieved ${pullRequests.length} PRs for ${owner}/${repo} in date range from API`)
|
|
||||||
|
|
||||||
const prCommits = filterCommits(commits, configuration.exclude_merge_branches)
|
const prCommits = filterCommits(commits, configuration.exclude_merge_branches)
|
||||||
|
|
||||||
core.info(`ℹ️ Retrieved ${prCommits.length} release commits for ${owner}/${repo}`)
|
core.info(`ℹ️ Retrieved ${prCommits.length} release commits for ${owner}/${repo}`)
|
||||||
|
|
||||||
// create array of commits for this release
|
// create array of commits for this release
|
||||||
const releaseCommitHashes = prCommits.map(commmit => {
|
const releaseCommitHashes = prCommits.map(commit => {
|
||||||
return commmit.sha
|
return commit.sha
|
||||||
})
|
})
|
||||||
|
|
||||||
|
let pullRequests: PullRequestInfo[]
|
||||||
|
if (releaseCommitHashes.length < 10) {
|
||||||
|
// fetch PRs based on commits instead (will get associated PRs per commit found)
|
||||||
|
const prsForReleaseCommits: Map<number, PullRequestInfo> = new Map()
|
||||||
|
for (const commit of prCommits) {
|
||||||
|
const result = await this.getForCommitHash(owner, repo, commit.sha, configuration.max_pull_requests)
|
||||||
|
result.forEach(pr => prsForReleaseCommits.set(pr.number, pr))
|
||||||
|
}
|
||||||
|
const dedupedPrsForReleaseCommits = Array.from(prsForReleaseCommits.values())
|
||||||
|
if (!includeOpen) {
|
||||||
|
pullRequests = dedupedPrsForReleaseCommits.filter(pr => pr.status !== 'open')
|
||||||
|
core.info(`ℹ️ Retrieved ${pullRequests.length} PRs for ${owner}/${repo} based on the release commit hashes`)
|
||||||
|
} else {
|
||||||
|
pullRequests = dedupedPrsForReleaseCommits
|
||||||
|
core.info(`ℹ️ Retrieved ${pullRequests.length} PRs for ${owner}/${repo} based on the release commit hashes (including open)`)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// fetch PRs based on the date range identified
|
||||||
|
const pullRequestsBetweenDate = await this.getBetweenDates(owner, repo, fromDate, toDate, configuration.max_pull_requests)
|
||||||
|
core.info(`ℹ️ Retrieved ${pullRequestsBetweenDate.length} PRs for ${owner}/${repo} in date range from API`)
|
||||||
|
|
||||||
// filter out pull requests not associated with this release
|
// filter out pull requests not associated with this release
|
||||||
const mergedPullRequests = pullRequests.filter(pr => {
|
const mergedPullRequests = pullRequestsBetweenDate.filter(pr => {
|
||||||
return releaseCommitHashes.includes(pr.mergeCommitSha)
|
return releaseCommitHashes.includes(pr.mergeCommitSha)
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -239,6 +277,8 @@ export class PullRequests {
|
|||||||
|
|
||||||
core.info(`ℹ️ Retrieved ${allPullRequests.length} total PRs for ${owner}/${repo}`)
|
core.info(`ℹ️ Retrieved ${allPullRequests.length} total PRs for ${owner}/${repo}`)
|
||||||
}
|
}
|
||||||
|
pullRequests = allPullRequests
|
||||||
|
}
|
||||||
|
|
||||||
// retrieve base branches we allow
|
// retrieve base branches we allow
|
||||||
const baseBranches = configuration.base_branches
|
const baseBranches = configuration.base_branches
|
||||||
@@ -247,7 +287,7 @@ export class PullRequests {
|
|||||||
})
|
})
|
||||||
|
|
||||||
// return only prs if the baseBranch is matching the configuration
|
// return only prs if the baseBranch is matching the configuration
|
||||||
const finalPrs = allPullRequests.filter(pr => {
|
const finalPrs = pullRequests.filter(pr => {
|
||||||
if (baseBranches.length !== 0) {
|
if (baseBranches.length !== 0) {
|
||||||
return baseBranchPatterns.some(pattern => {
|
return baseBranchPatterns.some(pattern => {
|
||||||
return pr.baseBranch.match(pattern) !== null
|
return pr.baseBranch.match(pattern) !== null
|
||||||
|
|||||||
Reference in New Issue
Block a user