isaacs / github

Just a place to track issues and feature requests that I have for github
2.21k stars 129 forks source link

Notify when merge conflict arises #224

Open OliverJAsh opened 10 years ago

OliverJAsh commented 10 years ago

My colleagues always have to ping me when there's a merge conflict.

This could be automated if GitHub notified the PR author whenever a merge conflict arises, saving my colleagues precious finger energy.

cirosantilli commented 10 years ago

+1

Mithgol commented 10 years ago

Is it different from #203 and how? (Do you mean such situations where the merge conflict arises not because of the pull request itself, but once the target branch moves on?)

OliverJAsh commented 10 years ago

@Mithgol Yes, that's what I mean. When the target branch (the branch I'm requesting a merge for) moves on and a conflict arises.

cirosantilli commented 9 years ago

The difficulty with this feature is of course performance: if you have 100 PRs, all have to be checked on each push to master.

It might also generate a huge number of notifications if an user has multiple PRs (I have ~50 on GitLab =( ). But true, a single conflict digest email could be sent per time chunk.

GitLab and likely GitHub cache the mergeability, and only update it when someone visits the PR show page.

Related discussion on GitLab for another feature that suffers from the same limitation: showing mergeability on the PR index: http://feedback.gitlab.com/forums/176466-general/suggestions/6472237-show-on-index-view-merge-requests-can-be-automatic

Of course, the email could be triggered only when the cache is updated by some visitor.

amcsi commented 8 years ago

+1

aiko0403 commented 8 years ago

+1

rmihael commented 8 years ago

+1

BenDTU commented 8 years ago

+1

mayurshivakumar commented 8 years ago

+1

arenaq commented 8 years ago

+1 (make it +20 for all developers here :D)

arenaq commented 8 years ago

@cirosantilli Of course it is performace issue and of course you can get spammed. But not all developers have so many pending MR's. For example, we have 1-2 in team, because we are constantly doing code reviews and merging them. But even if it was more, I would not consider it spamming. Either way it would be nice to have this option in settings so anyone could either turn it on or turn it off (perhaps both options - spamming one and periodically).

enumag commented 8 years ago

Need this too. And even better than a notification would be an automatically added label / icon / whatever on the PR list page.

kf6kjg commented 8 years ago

Heavy annoyance: too many times wondering why my reviewer-approved PR hasn't been merged in and come to find out that the target moved on and those very busy people with write access have been skipping it because it suddenly developed conflicts. The same people are too busy to spend the time either merging it themselves or taking the time to send an email - such is the system's job.

gitresolve commented 7 years ago

@OliverJAsh GitResolve does this. It determines if there is a merge conflict and notifies the users who can fix the them whenever conflicts arise. A shareable link with a preview of the merge conflict is also provided. Here's how it works: https://gitresolve.com/how-it-works Check it out.

arenaq commented 7 years ago

@OliverJAsh And I though it is a plugin to GitLab. Such a commercial :D

arenaq commented 7 years ago

@OliverJAsh Can you explain how is this thread solved?

OliverJAsh commented 7 years ago

Oops, meant to unsubscribe, not close.

gitresolve commented 7 years ago

RE: And I though it is a plugin to GitLab. Such a commercial :D It's actually a service, hoping people find it useful :)

RE: Need this too. And even better than a notification would be an automatically added label / icon / whatever on the PR list page. Updated the service so it now automatically labels pull request with a "conflicted" label when it is unmergeable. The label also automatically gets removed when the pull request returns to a mergeable state. The label and notify settings are both configurable via a gitresolve.yml settings file so you can selectively turn on and off the behaviors.

arenaq commented 7 years ago

@gitresolve You are talking about label. That would mean to start using the service instead of/with gitlab, right? Why to use two services when you can use just one?

Only these notifications make sense but I doubt that any Ops team would like to install it just for notifications about conflicts. :)

gitresolve commented 7 years ago

@arenaq RE: GitLab. Oh ic. Sorry, yup, this only works with GitHub :( It is completely fair that an ops team will not be ok with installing it just for notifications. It is starting with merge conflicts and will hopefully automate more mundane tasks later. Thanks for the feedback.

GrahamTheCoder commented 7 years ago

@gitresolve It's really great that you're tackling this problem. I think this could be really helpful.

I'm looking for a solution that goes further, and will notify about PRs that would conflict with each each other once the other was merged. You can cut down the apparent n x n complexity significantly in the average case by maintaining a dictionary of file path to list of {time last changed, PR that changed it}, then only check those particular file/PR combos for conflicts when they change again. Bonus points: Do it for branches that don't have PRs yet.

If anyone knows of someone who's already solved this, on GitHub or as a local application, let me know.

kpedro88 commented 7 years ago

+1 Since this info is cached by GitHub, it should be included as part of the state of the pull request and accessible via the API.

hkraji commented 7 years ago

+1

eddie-git commented 7 years ago

+1

natashawang commented 7 years ago

+1

nesterchung commented 7 years ago

+1

ldu-iRiS commented 7 years ago

+1

TwanVermeulen commented 7 years ago

+1

benjaminaaron commented 7 years ago

+1

AndreKelling commented 6 years ago

+1

surfaceowl commented 6 years ago

+1 - how can this not be an optional feature for teams to toggle on and off?

abartl commented 6 years ago

+1

johnmosesman commented 6 years ago

+1. Without this there are several back and forth interruptions between the author and reviewer—and the author has no way to know that they need to update their PR.

thomaswhyyou commented 6 years ago

+1

JackPotte commented 6 years ago

+1

vojtapol commented 6 years ago

+1, if you are worried about performance, make it configurable, disabled by default, per user, whatever

pqtrang commented 6 years ago

+1

eperales commented 6 years ago

+1

benja83 commented 6 years ago

+1

Colin-R commented 6 years ago

+1

jonvnieu commented 6 years ago

+1

GautamGupta commented 6 years ago

This is now exposed via the API using the mergeable attribute: https://developer.github.com/v3/pulls/#response-1 (on a per-PR basis) or https://developer.github.com/v4/enum/mergeablestate/

ukjinlee commented 6 years ago

+1

mbana commented 6 years ago

+1

howdou commented 6 years ago

+1

abinoda commented 5 years ago

I'm the developer of Pull Reminders, a Slack bot for GitHub that provides real-time DM notifications.

I've tried adding merge conflict notifications but found that it was basically unfeasible given that mergeability status is only calculated by GitHub when a pull request is loaded.

This means that it generally requires a minimum of two API requests to get the mergeability status of a pull request. I did a longer write-up here.

abinoda commented 5 years ago

Update: Pull Reminders now offers merge conflict notifications for Slack 🕺

Kallin commented 5 years ago

Update: Pull Reminders now offers merge conflict notifications for Slack 🕺

I don't see this listed as a feature on pullreminders. Is it available?

abinoda commented 5 years ago

@Kallin Where are you looking? It is available under personal notification settings:

Screen Shot 2019-03-21 at 3 58 14 PM
KyleWiering commented 4 years ago

What about just auto-closing the request when conflict arises? Then you'd limit the amount of churn to relevant PR's

And notifications for closed PR's already work