slack Ability to omit Draft Pull Request

Ability to omit Draft Pull Request

Open justinperkins opened this issue 5 years ago • 62 comments

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?

Feb 20 '19 22:02 justinperkins

@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

Feb 20 '19 22:02 abinoda

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.

Feb 20 '20 22:02 stale[bot]

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.

Feb 21 '20 03:02 justinperkins

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.

Mar 02 '20 21:03 nitrocode

draft is a boolean state on the pull request object

Mar 03 '20 13:03 justinperkins

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.

Apr 28 '20 00:04 mbainter

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

Apr 28 '20 11:04 justindotpub

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.

Apr 28 '20 15:04 gareth

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

Apr 28 '20 18:04 gugaiz

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.

Apr 28 '20 20:04 justinperkins

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.

May 14 '20 06:05 andersarpi

Would love to see this happen as well 👍

May 18 '20 09:05 doutatsu

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.

Jun 02 '20 10:06 charlax

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.