Skip to content

[GH Actions] Request review of automerge PR if there is a merge conflict #1709

Closed

Description

@arielgreen

If you haven’t already, check out our contributing guidelines for onboarding!


Problem

Typically, PRs that have the automerge label don't need to be manually reviewed by anyone. In those cases, we don't want to request a review. However, since we'll be using automerge PRs to CP PRs to staging and/or production, we'll have more cases where it can't be automatically merged because of a merge conflict. In these cases, we need to request a reviewer on the PR so that the reviewer can manually resolve the merge conflict.

Why is this important?

This will be a pre-requisite of our Expensify.cash CP flows.

Solution

If this step of the automerge workflow exits with a nonzero exit code, add pullerbear as a reviewer on the PR.

Apply to this posting on Upwork: https://www.upwork.com/jobs/~01977821838810f422
https://github.com/Expensify/Expensify/issues/155154

Activity

  1. attaradev commented on Mar 10, 2021

    @attaradev
    Contributor

    Proposed solution
    To determine the mergeability of the PR, the mergeable field on the PR is a Boolean that indicates if the branch can be merged, and mergeable_state will indicate if it's clean, dirty, or unstable. So, I will improve the automerge workflow by checking if the PR is mergeable, and merge or request a review.
    The Status API can also be used to mark a commit on the PR that will be used to request for manual review.

  2. attaradev commented on Mar 11, 2021

    @attaradev
    Contributor

    Alternate solution:
    Add id for this step so that I can check the exitcode of the step to add pullbear as reviewer or not like this as suggested in the issue description.

  3. roryabraham commented on Mar 11, 2021

    @roryabraham
    Contributor

    @mikeattara I like your first solution that checks the mergeable field! Proposal looks good, but just for clarity the correct actor is Expensify/pullerbear, not pullbear

  4. attaradev commented on Mar 11, 2021

    @attaradev
    Contributor

    Alright. Thanks for the clarification @roryabraham

  5. arielgreen commented on Mar 11, 2021

    @arielgreen
    ContributorAuthor

    Sending over an offer in Upwork now, @mikeattara .

  6. attaradev commented on Mar 11, 2021

    @attaradev
    Contributor

    Thanks. @arielgreen

  7. attaradev commented on Mar 16, 2021

    @attaradev
    Contributor

    Update
    Currently, all pull requests require at least one approving review to be merged so I added a check on the automerge workflow for mergeability and mergeable_state.

    • Approve pull request if there is no merge conflict.
    • Merge pull request if there is no merge conflict and mergeable state is clean.

    NB: Expensify/pullbearer is automatically added as a reviewer for all pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions