Skip to content
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

Support for rebase merging #106

Closed
TimonVS opened this issue Dec 20, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@TimonVS
Copy link
Collaborator

commented Dec 20, 2018

Currently rebase merging is not supported because Release Drafter checks for merge commits which don't exist when using rebase merging. At our company we're unable to use Release Drafter because we use rebase merging. Adding support for rebase merging requires a bit of a rewrite since it shouldn't rely on commits anymore, instead we can use the search endpoint (or search field in the GraphQL API) to fetch pull requests.

Here's an example query:

{
  search(type: ISSUE, query: "type:pr merged:>=2018-11-10 repo:toolmantim/release-drafter", first: 10) {
    nodes {
      ... on PullRequest {
        title
        number
        author {
          login
        }
        mergedAt
      }
    }
    pageInfo {
      endCursor
      hasNextPage
    }
  }
}

Would you be interested in this @toolmantim? I could start working on it tomorrow and next week :)

@toolmantim

This comment has been minimized.

Copy link
Owner

commented Dec 20, 2018

Hi @TimonVS! We're actually hitting API limits with the current method (if there's lots of PRs to fetch), so switching to GraphQL would be better. Would be happy to switch to this.

If we could ensure that #98 is still possible though (adding directly-pushed-to-master commits), it'd be much appreciated.

@TimonVS

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 21, 2018

It should drastically reduce the amounts of requests made. Currently I'm not sure if #98 will be possible with this approach, but I think it'd be a very useful feature to have. I've asked help for this in the Probot Slack :)

@toolmantim

This comment has been minimized.

Copy link
Owner

commented Dec 21, 2018

For #98, perhaps you might need to query both the commits and the PRs? Either separately, or in the same query?

@TimonVS

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 22, 2018

That could work for merge commits and squash merging, but for rebase merging I will not be able to discern commits to master and commits that belong to a rebase merge. GitHub does however have a way to link the commits of a rebase merge to the PR as can be seen in the interface:
pr-number
But I'm unsure if they expose it in the API in some way (I couldn't find it). I haven't received a response on the Probot Slack yet, I will try to reach out to the API team directly.

@toolmantim

This comment has been minimized.

Copy link
Owner

commented Dec 22, 2018

Ah… I see! I've spotted associatedPullRequests on Ref in the GraphQL API before, but I'm guessing it doesn't do what we need.

@TimonVS

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 8, 2019

Just wanted to let you know that I haven't forgotten about this. I've posted a question to the community forum here: https://github.community/t5/GitHub-API-Development-and/Get-pull-request-associated-with-a-commit/m-p/16764 but there's no solution yet.

@TimonVS

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 12, 2019

It's been quite awhile, but it seems that GitHub has recently added a new field which makes it possible to retrieve the associated pull requests from a commit: https://developer.github.com/v4/changelog/2019-03-08-schema-changes/. I'll get on this tomorrow.

@toolmantim

This comment has been minimized.

Copy link
Owner

commented Mar 12, 2019

🎉

@TimonVS TimonVS referenced this issue Apr 3, 2019

Merged

Add support for rebase merging #164

11 of 11 tasks complete
@TimonVS

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 29, 2019

This should be solved now with #164 being merged :)

@TimonVS TimonVS closed this Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.