servo / highfive

Github hooks to provide an encouraging atmosphere for new contributors
Mozilla Public License 2.0
254 stars 58 forks source link

Add S-needs-rebase label when homu posts "Merge conflict" comment #24

Open jdm opened 9 years ago

jdm commented 9 years ago

And remove S-awaiting-merge, too.

magni- commented 8 years ago

This is done, right? For example here: https://github.com/servo/servo/pull/9911#issuecomment-200660508

Doesn't look the the label gets properly removed though, should I create an issue for that?

magni- commented 8 years ago

Hmm, we already have this, but it doesn't seem to work since mergeable is returned as null.

    # If mergeable is null, the data wasn't available yet. It would be nice to try to fetch that
    # information again.
    if payload["action"] == "synchronize" and payload['pull_request']['mergeable']:
        if "S-needs-rebase" in labels:
            api.remove_label("S-needs-rebase")
KiChjang commented 8 years ago

@magni- I don't think it's the same issue. This is when highfive only says "Merge conflict", and it happens when a PR is r+'d and in the queue, but the head of queue PR, after merging, caused a merge conflict on the PR in queue.

magni- commented 8 years ago

You mean homu, right? If it's a PR comment like any other, it's just a matter or adding another elif just like this one then, right?

        elif 'Please resolve the merge conflicts' in msg:
            remove_if_exists("S-awaiting-merge")
            api.add_label("S-needs-rebase")

(or changing it to check for erge conflict, if that's not considered too hacky :stuck_out_tongue:)