-
Notifications
You must be signed in to change notification settings - Fork 317
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
Add support for rebase merging #164
Changes from all commits
cac3724
ea4f0d1
01ada87
a7ed366
27e8a3b
f302661
ce7e248
6a957bd
99565ad
4db2242
3e3a5ee
1cd5a48
d6edb66
999fe73
99c6e84
6494653
ee5b782
6ff1e70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,78 +1,99 @@ | ||
const log = require('./log') | ||
const paginate = require('./pagination') | ||
const _ = require('lodash') | ||
|
||
module.exports.findCommits = async ({ app, context, branch, lastRelease }) => { | ||
const findCommitsWithAssociatedPullRequestsQuery = /* GraphQL */ ` | ||
query findCommitsWithAssociatedPullRequests( | ||
$name: String! | ||
$owner: String! | ||
$branch: String! | ||
$since: GitTimestamp | ||
$after: String | ||
) { | ||
repository(name: $name, owner: $owner) { | ||
ref(qualifiedName: $branch) { | ||
target { | ||
... on Commit { | ||
history(first: 100, since: $since, after: $after) { | ||
totalCount | ||
pageInfo { | ||
hasNextPage | ||
endCursor | ||
} | ||
nodes { | ||
id | ||
message | ||
author { | ||
name | ||
user { | ||
login | ||
} | ||
} | ||
associatedPullRequests(first: 5) { | ||
nodes { | ||
title | ||
number | ||
author { | ||
login | ||
} | ||
labels(first: 10) { | ||
nodes { | ||
name | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
` | ||
|
||
module.exports.findCommitsWithAssociatedPullRequests = async ({ | ||
app, | ||
context, | ||
branch, | ||
lastRelease | ||
}) => { | ||
const { owner, repo } = context.repo() | ||
const variables = { name: repo, owner, branch } | ||
const dataPath = ['repository', 'ref', 'target', 'history'] | ||
|
||
let data | ||
if (lastRelease) { | ||
log({ | ||
app, | ||
context, | ||
message: `Comparing commits ${lastRelease.tag_name}..${branch}` | ||
message: `Fetching all commits for branch ${branch} since ${ | ||
lastRelease.published_at | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks funky There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah looks a bit weird, it happens because the max line length is reached. There's not much I can do about it except increasing the line length 😬. I personally don't really mind though. |
||
}` | ||
}) | ||
// Only supports up to 250 commits | ||
return context.github.repos | ||
.compareCommits( | ||
context.repo({ | ||
base: lastRelease.tag_name, | ||
head: branch | ||
}) | ||
) | ||
.then(res => res.data.commits) | ||
|
||
data = await paginate( | ||
context.github.graphql, | ||
findCommitsWithAssociatedPullRequestsQuery, | ||
{ ...variables, since: lastRelease.published_at }, | ||
dataPath | ||
) | ||
} else { | ||
log({ app, context, message: `Fetching all commits for branch ${branch}` }) | ||
return context.github.paginate( | ||
context.github.repos.listCommits.endpoint.merge( | ||
context.repo({ | ||
sha: branch, | ||
per_page: 100 | ||
}) | ||
) | ||
) | ||
} | ||
} | ||
|
||
module.exports.extractPullRequestNumber = commit => { | ||
// There are two types of GitHub pull request merges, normal and squashed. | ||
// Normal ones look like 'Merge pull request #123' | ||
// Squashed ones have multiple lines, first one looks like 'Some changes (#123)' | ||
const match = commit.commit.message | ||
.split('\n')[0] | ||
.match(/\(#(\d+)\)$|^Merge pull request #(\d+)/) | ||
return match && (match[1] || match[2]) | ||
} | ||
|
||
const findPullRequest = ({ context, number }) => { | ||
return ( | ||
context.github.pullRequests | ||
.get(context.repo({ number: number })) | ||
.then(res => res.data) | ||
// We ignore any problems, in case the PR number pulled out of the commits | ||
// are bonkers | ||
.catch(() => false) | ||
) | ||
} | ||
|
||
module.exports.findPullRequests = ({ app, context, commits }) => { | ||
if (commits.length === 0) { | ||
return Promise.resolve([]) | ||
data = await paginate( | ||
context.github.graphql, | ||
findCommitsWithAssociatedPullRequestsQuery, | ||
variables, | ||
dataPath | ||
) | ||
} | ||
|
||
const pullRequestNumbers = commits | ||
.map(module.exports.extractPullRequestNumber) | ||
.filter(number => number) | ||
|
||
log({ | ||
app, | ||
context, | ||
message: `Found pull request numbers: ${pullRequestNumbers.join(', ')}` | ||
}) | ||
|
||
const pullRequestPromises = pullRequestNumbers.map(number => | ||
findPullRequest({ context, number }) | ||
const commits = _.get(data, [...dataPath, 'nodes']) | ||
const pullRequests = _.uniqBy( | ||
_.flatten(commits.map(commit => commit.associatedPullRequests.nodes)), | ||
'number' | ||
) | ||
|
||
return ( | ||
Promise.all(pullRequestPromises) | ||
// Filter out PR lookups that failed | ||
.then(prs => prs.filter(pr => pr)) | ||
.catch(() => []) | ||
) | ||
return { commits, pullRequests } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
const _ = require('lodash') | ||
|
||
/** | ||
* Utility function to paginate a GraphQL function using Relay-style cursor pagination. | ||
* | ||
* @param {Function} queryFn - function used to query the GraphQL API | ||
* @param {string} query - GraphQL query, must include `nodes` and `pageInfo` fields for the field that will be paginated | ||
* @param {Object} variables | ||
* @param {string[]} paginatePath - path to field to paginate | ||
*/ | ||
async function paginate(queryFn, query, variables, paginatePath) { | ||
const nodesPath = [...paginatePath, 'nodes'] | ||
const pageInfoPath = [...paginatePath, 'pageInfo'] | ||
const endCursorPath = [...pageInfoPath, 'endCursor'] | ||
const hasNextPagePath = [...pageInfoPath, 'hasNextPage'] | ||
const hasNextPage = data => _.get(data, hasNextPagePath) | ||
|
||
let data = await queryFn(query, variables) | ||
|
||
if (!_.has(data, nodesPath)) { | ||
throw new Error( | ||
"Data doesn't contain `nodes` field. Make sure the `paginatePath` is set to the field you wish to paginate and that the query includes the `nodes` field." | ||
) | ||
} | ||
|
||
if ( | ||
!_.has(data, pageInfoPath) || | ||
!_.has(data, endCursorPath) || | ||
!_.has(data, hasNextPagePath) | ||
) { | ||
throw new Error( | ||
"Data doesn't contain `pageInfo` field with `endCursor` and `hasNextPage` fields. Make sure the `paginatePath` is set to the field you wish to paginate and that the query includes the `pageInfo` field." | ||
) | ||
} | ||
|
||
while (hasNextPage(data)) { | ||
const newData = await queryFn(query, { | ||
...variables, | ||
after: _.get(data, [...pageInfoPath, 'endCursor']) | ||
}) | ||
const newNodes = _.get(newData, nodesPath) | ||
const newPageInfo = _.get(newData, pageInfoPath) | ||
|
||
_.set(data, pageInfoPath, newPageInfo) | ||
_.update(data, nodesPath, d => d.concat(newNodes)) | ||
} | ||
|
||
return data | ||
} | ||
|
||
module.exports = paginate |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be better only to require what you need?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performance wise it shouldn't really matter I think? I like the convenience of having all of Lodash under a single char 😅