Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[wip] feat: Add the ability to auto merge PRs #394

Closed
wants to merge 1 commit into from

Conversation

sambhav
Copy link
Contributor

@sambhav sambhav commented Oct 5, 2020

This PR adds a couple of things -

  1. A new event type listener for status.* this allows us to listen to status change events so that we can fire the merge action on status change events.
  2. A validator for status.*. This validator resolves the commit related to the status to relevant PRs on the repository. There is a checks setting option it takes to filter our PRs with failing checks. (This is optional as some checks may not be required to do merge)
  3. Adds support for status* events to the merge actions.
  4. Checks that the pull request is not in a blocked mergeable_state before trying to merge. This ensures that the PR is ready to merge (passes required checks including status, review and other branch protection checks)
  5. Fixes broken merge action. merge action was missing the name parameter.

Fixes #395

Fixes #386

TODO: Update docs and add tests.

@sambhav sambhav changed the title Add the ability to auto merge PRs feat: Add the ability to auto merge PRs Oct 5, 2020
@sambhav sambhav force-pushed the merge-on-status branch 5 times, most recently from 8c6977b to 9fc37e7 Compare October 5, 2020 04:14
@sambhav
Copy link
Contributor Author

sambhav commented Oct 5, 2020

@shine2lay @jusx for some reason the tests are failing with node 8.8 but work with node 13.10 - thoughts? (Which is why I updated the node version)

@jusx
Copy link
Member

jusx commented Oct 6, 2020

@samj1912 Could it be that the code base uses JS features that is not supported in node 8.

@jusx
Copy link
Member

jusx commented Oct 6, 2020

@samj1912 I like the approach and it looks good to me. I noticed a TODO note in the description. Will you be adding the docs and test as part of this PR? Thanks in advance.

@shine2lay Please take a look at this. It looks like it will work well and will resolve #395 and probably #386

@sambhav sambhav changed the title feat: Add the ability to auto merge PRs [wip] feat: Add the ability to auto merge PRs Oct 6, 2020
@sambhav
Copy link
Contributor Author

sambhav commented Oct 7, 2020

Note - I have a much simpler WIP in mind. But it relies on #402 being closed.

@sambhav
Copy link
Contributor Author

sambhav commented Oct 7, 2020

Closing this for now - will add a better version of this soon.

@sambhav sambhav closed this Oct 7, 2020
@mergeable
Copy link

mergeable bot commented Oct 7, 2020

❌ Error Occurred while executing an Action

If you believe this is an unexpected error, please report it on our issue tracker: https://github.com/mergeability/mergeable/issues/new

Error Details

Error: Merge failed! error : HttpError: 2 of 2 required status checks are expected. At least 1 approving review is required by reviewers with write access.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants