search

z0al / probot-review-me

Decide when a pull request is ready for review based on its statuses
MIT License
16 stars 0 forks source link

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

Open johlju opened 6 years ago

johlju commented 6 years ago

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 commented 6 years ago

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

johlju commented 6 years ago

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/

johlju commented 6 years ago

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 https://github.com/z0al/probot-review-me/issues/4#issuecomment-417237137 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 commented 6 years ago

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 commented 6 years ago

Yes, it was like that I meant.

z0al commented 6 years ago

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

johlju commented 6 years ago

Ah, with multi-rules, do you mean when issue #1 is resolved? I updated my suggest config in https://github.com/z0al/probot-review-me/issues/1#issuecomment-394665843 for a proposal how it could be.