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 "This branch has conflicts that must be resolved" #3

Open
johlju opened this issue Aug 7, 2018 · 7 comments
Open

Support "This branch has conflicts that must be resolved" #3

johlju opened this issue Aug 7, 2018 · 7 comments
Labels
enhancement New feature or request

Comments

@johlju
Copy link

johlju commented Aug 7, 2018

It would be nice if a PR has merge conflicts, that could be detected as a "context" as well.

when:
  merge-conflicts: success

Though, would work if there was another app that detected this and added another status check that this app could be detected.

@z0al
Copy link
Owner

z0al commented Aug 7, 2018

This is actually a very useful and practical feature that this app must have, thanks

@johlju
Copy link
Author

johlju commented Aug 11, 2018

Fir reference, from GitHub support:

You could use our GraphQL API to fetch a pull request and it's mergeStateStatus as well as its
mergeable attribute: https://developer.github.com/v4/object/pullrequest/

mergeable is a field with the return type of MergeableState, and MergeableState is an enum type. If it
has a value of CONFLICTING, this would indicate the pull request cannot be merged due to merge
conflicts: https://developer.github.com/v4/enum/mergeablestate/#conflicting

If the pull request has a mergeable value of CONFLICTING, you could check the
mergeStateStatus to get more detailed information about the current pull request merge state status:
https://developer.github.com/v4/enum/mergestatestatus/

@z0al z0al mentioned this issue Aug 17, 2018
4 tasks
@johlju
Copy link
Author

johlju commented Aug 30, 2018

Detection of merge conflicts should be an opt-in, so it should be separate from statuses, maybe this should be a repo wide setting instead. See #4 (comment) for example when it should be opt-in.

Maybe it more accurate to have this setting

detectMergeConflicts: yes
when:
  continuous-integration/travis-ci/pr: success
  wip: success

If detectMergeConflicts is set to yes/true then that will always override any other status under when.

@z0al
Copy link
Owner

z0al commented Aug 31, 2018

If detectMergeConflicts is set to yes/true then that will always override any other status under when.

What do you mean by that?

I was thinking of the followings (but now this won't work when we introduce multi-rules):

  1. If detectMergeConflicts is set to true

    • if there are merge conflicts then ReviewMe should simply do the same as it would do if states matching failed (but without actually checking statues)
    • if there are no conflicts then ReviewMe should continue its work normally
  2. If detectMergeConflicts is set to false

Just continue to work the way it does right now.

@johlju
Copy link
Author

johlju commented Sep 4, 2018

Yes, it was like that I meant.

@z0al
Copy link
Owner

z0al commented Sep 4, 2018

I'm still not sure how that would work with multi-rules!

@johlju
Copy link
Author

johlju commented Sep 4, 2018

Ah, with multi-rules, do you mean when issue #1 is resolved? I updated my suggest config in #1 (comment) for a proposal how it could be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants