integrations / slack

Bring your code to the conversations you care about with the GitHub and Slack integration
https://slack.github.com/
MIT License
3.05k stars 479 forks source link

Ability to omit Draft Pull Request #795

Open justinperkins opened 5 years ago

justinperkins commented 5 years ago

Now that we can make draft PRs (🎉), we've created the desire to keep those draft PRs out of our channels until they've been made official. Thoughts?

abinoda commented 5 years ago

@justinperkins This shouldn't be too hard of a pull request to submit. I might give it a shot myself if more folks are interested.

Related to this, I'm working on Draft support in my 3rd-party Slack app and running into some issues due to there not being a webhook event signaling that a pull request is no longer a draft: https://github.com/pullreminders/backlog/issues/124#issuecomment-464276841

stale[bot] commented 4 years ago

Is this still relevant? If so, just comment with any updates and we'll leave it open. Otherwise, if there is no further activity, it will be closed.

justinperkins commented 4 years ago

Lame Stalebot, this is very much still relevant and a problem. Draft PRs sending PR ready for review announcements is noise and takes away from the "ready for review" state change on the PR.

nitrocode commented 4 years ago

Is a pull_request of type draft even accessible? I don't see a key for it exposed in the github api PullRequestEvent webhook unless the documentation is outdated.

justinperkins commented 4 years ago

draft is a boolean state on the pull request object

mbainter commented 4 years ago

I'm curious if this is a universal problem. The language of the documentation suggests it only publishes a draft PR if it is marked as "ready for review". This happens to my team, but I think it might be because we have a CODEOWNERS file which automatically sets a required reviewer even on draft PRs.

justindotpub commented 4 years ago

I think it’s universal. We don’t have a CODEOWNERS file and drafts still show up in Slack.

gareth commented 4 years ago

The language being referenced is:

pulls - New or merged pull requests, as well as draft pull requests marked "Ready for Review"

New draft pull requests are still new pull requests, and so are covered both by the first half of the sentence (when the PR is first created) and the second half of the sentence (when marked "Ready for Review").

But the language confused me and my organisation the same way it did @mbainter.

gugaiz commented 4 years ago

I am running my tests on github actions but I don't want them to run on draft PR, so I added this to my action types: [review_requested, ready_for_review] but it won't run when the PR is ready and there is someone on the list of requested reviews so not sure if that is a expected behavior and if it has to do with this topic

justinperkins commented 4 years ago

pulls - New or merged pull requests, as well as draft pull requests marked "Ready for Review"

New draft pull requests are still new pull requests, and so are covered both by the first half of the sentence (when the PR is first created) and the second half of the sentence (when marked "Ready for Review").

I agree with the latter, a Draft PR marked ready for review should trigger a notification (it does), but a new draft PR should at least have a separate subscription channel than new PRs ready for review. As it is, it is noise in the notification channels and leads to confusion and training team members to ignore messages.

andersarpi commented 4 years ago

Would like to add that this is a universal problem that we are facing too. Getting a "PR opened" notification in Slack for a draft PR is definitely something for which I would like a disable option.

doutatsu commented 4 years ago

Would love to see this happen as well 👍

charlax commented 4 years ago

I'd be happy to take a stab at this. I think it would make sense for Draft PR to be excluded by default from the pulls subscription, and to create another subscription one pulls:draft for drafts? That would be a breaking change btw.

There is already a similar distinction between commits and commits:all.

sfdye commented 4 years ago

Please, I would love to see this being implemented. The draft mode was designed to reduce noise for code owners, so the slack integration should respect that as well.

sfdye commented 4 years ago

@abinoda FWIW I think the webhook events are available now

image

olafkotur commented 4 years ago

Any updates on this?

abinoda commented 4 years ago

@sfdye Thanks for the ping. I am not involved with the GitHub Slack integration in any way however.

joeydong commented 4 years ago

+1

pkiv commented 4 years ago

Big +1!

JamesGelok commented 4 years ago

@ abinoda FWIW I think the webhook events are available now

That link is deprecated.

WebHook events are located here now

WyriHaximus commented 3 years ago

Any news on this? Still getting spam messages from draft PR's everytime someone opens one

fred-tyemill commented 3 years ago

💯 , this would be great to have.

avcconti commented 3 years ago

+1 Draft PRs are adding too much noise into slack channels.

adithshenoy commented 3 years ago

+1 Reduce noise 🔕

jty-cva commented 3 years ago

+1 this will be a great feature to have to reduce noise

shyamal-anadkat commented 3 years ago

+1

2121159i commented 3 years ago

+1 for less noise

tariqporter commented 3 years ago

+1

ngie-eign commented 3 years ago

I would really appreciate if this could be merged. Displaying status for draft PRs in Slack goes against the spirit of draft PRs IMHO.

david-m-globant commented 3 years ago

+1

justinperkins commented 3 years ago

Over 2 years old! @gauravsaralMs, you're actively working on the repo, can you work this into your near future?

amitkumariiit commented 3 years ago

Hi @justinperkins and all, this is in our backlog and soon will omit the draft PR notification.

k-utsumi commented 2 years ago

Maybe this has already been released resolved. https://github.com/integrations/slack/pull/1109

By the way, I'd like to send Draft PR comments to Slack. Is that possible...? If I can't, I wouldn't want to use Draft PR.

markmesscu commented 2 years ago

By the way, I'd like to send Draft PR comments to Slack. Is that possible...? If I can't, I wouldn't want to use Draft PR.

Also wondering if it is possible to send Draft PR comments to Slack...

tonglil commented 2 years ago

Seems like draft PRs are not sent to Slack any more, but it's something we want.

sjmueller commented 2 years ago

Likewise, please add back Draft PRs as it was in the original integration!

WyriHaximus commented 2 years ago

Kindly leave them out, or provide a configuration option for the users. Not everyone wants them...

sjmueller commented 2 years ago

Kindly leave them out

@WyriHaximus I don't understand leading with this sentiment -- we used to have it and was taken away -- simply make it an option that's not enabled by default and everyone is happy

WyriHaximus commented 2 years ago

@sjmueller Because we got spammed by draft PR's that added so much noise they drowned the PR's that did need attention. And now that's it is finally removed, there are requests to put them back in. So yes I'm going for the kind approach. Following with a:

or provide a configuration option for the users

sjmueller commented 2 years ago

I'm still not understanding why the choice was made in the first place to remove it completely, when nearly every other entity is configurable.

Perhaps that's the reason why lots of people on both sides have voiced their preferences? Was there a limitation of the code or the github hooks that make it particularly challenging at the time?

WyriHaximus commented 2 years ago

I'm still not understanding why the choice was made in the first place to remove it completely, when nearly every other entity is configurable.

I fully agree, in retrospect that should have been done.

replygirl commented 2 years ago

We need draft PR announcements to let people know when working target branches are available to them

sjmueller commented 2 years ago

hi @ashokirla is there any update as to when draft PRs will be configurable, so that we have the ability to get notified?

amiantos commented 2 years ago

Kinda hilarious this was yanked out and not made optional. It's actually really annoying that draft PR comments were working and now they aren't anymore.

Here's another issue: https://github.com/integrations/slack/issues/1320

justinperkins commented 2 years ago

It's actually really annoying that draft PR comments were working

What does "working" mean to you?

sjmueller commented 2 years ago

What kind of gaslighting is this?

"Working" means you could get notified in Slack on Draft PR activity.

Many devs have expressed this to be critical functionality that was previously available, and now it's not.

amiantos commented 2 years ago

It's actually really annoying that draft PR comments were working

What does "working" mean to you?

Like others (assumably), we came to rely on getting all PR comments in our Slack. It was a good way to get a solid overview of all the activity on our team, and a way for team members to see if they had insight to offer during PR discussions without being explicitly tagged. Then one day I noticed we were no longer getting draft PR comments in our Slack. My gut instinct was "must be a bug, GitHub having issues sending notifications".

Now they've been turned off, with no option to turn them back on. This strikes me as annoying: removing behavior that was there, and not giving us the option of turning it back on. It seems to me like a mistake was made, only listening to the people who were complaining, without considering all the silent people who weren't complaining.

justinperkins commented 2 years ago

Interesting. For my team we found the draft PR notifications to be more noise than signal, we don't care about PRs until they're ready to be reviewed. We already have commit notifications flowing in that indicates activity, the draft PRs on top of it are just superfluous to us. That's why I created this issue.

ngie-eign commented 2 years ago

Being able to tune whether or not draft pr comments show up in a slack channel seems like the right way to go about achieving the wants of both parties. Defaulting to off seems like a good default, but I’m biased with that vote…

k-utsumi commented 2 years ago

For example, I would like to quote draft PR comment or comment in draft pr comment thread with mention.