Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There are too many bot comments #5611

Closed
domenic opened this issue Apr 19, 2017 · 23 comments
Closed

There are too many bot comments #5611

domenic opened this issue Apr 19, 2017 · 23 comments
Labels

Comments

@domenic
Copy link
Member

domenic commented Apr 19, 2017

Opening a pull request will generate the following:

1. An edit to your original post advertising Reviewable (see #5590)
2. A notification that the tests are available on w3c-test.org. (This also appears to get re-posted after every push.)
3. wpt-pr-bot notifying reviewers
4. A notification that lint passed
5. A post with Firefox results
6. A post with Chrome results
7. A post telling me about code coverage (maybe this is testing harness coverage or something!?)

Of these, only 3, 5, and 6 are valuable.

3 could be eliminated by using GitHub's request-a-review feature.

2 is redundant with 5 and 6, but if people think getting it in a couple minutes before 5 and 6 hit is valuable, then it could edit the original post instead.

1, 4, and 7 are completely useless and spammy.

A workaround for 7 is that you can block the @codecov-io GitHub user, but you cannot use the same workaround for others, as blocking @wpt-pr-bot prevents you from being asked for reviews and blocking @w3c-bots prevents you from seeing 5 and 6.

@domenic domenic added the infra label Apr 19, 2017
@zcorpan
Copy link
Member

zcorpan commented Apr 26, 2017

@gsnedders @jgraham can the linter and codecov things only comment if they have something useful to say? i.e. don't add a new comment saying "lint passed" or "code coverage not changed". (I'm assuming the code coverage thing can sometimes have something useful to say, but I haven't seen anything so far, and I don't know what it does...)

@annevk
Copy link
Member

annevk commented Apr 26, 2017

@zcorpan yeah, those two are being addressed. I think code coverage is already addressed, but you might need to rebase first.

@tobie
Copy link
Contributor

tobie commented Apr 26, 2017

3 could be eliminated by using GitHub's request-a-review feature.

Agreed. I've offered to do that a bunch of times already, but @jgraham is against. Happy to fix it when there's consensus to do so (it should be a trivial change).

@gsnedders
Copy link
Member

3 could be eliminated by using GitHub's request-a-review feature.

The problem with this is you don't get a notification for it, and unless you're regularly clicking through to https://github.com/pulls/review-requested you won't notice anything new.

@tobie
Copy link
Contributor

tobie commented Apr 26, 2017

Really? That's not my experience of it (iirc) nor what the doc says. Am I missing something?

@domenic
Copy link
Member Author

domenic commented Apr 26, 2017

The problem with this is you don't get a notification for it,

That's not true? At least you get an email for it. My inbox has, for example:

@annevk requested your review on: whatwg/infra#118 Define insert for lists.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@gsnedders
Copy link
Member

Mea culpa. I was sure I'd never got any notification, but maybe I'm just not paying enough attention. And I never look at my GitHub email. :)

@tobie
Copy link
Contributor

tobie commented May 2, 2017

Is there a plan to address this? This noise hurts a lot more than it helps at this point. It would be nice if we could have a concerted effort not to spam the hell out of the PR threads.

A pattern that works really well that I use in pr-preview is to add links in the OP instead of adding a new comment. E.g.: the preview and diff links at the bottom of whatwg/webidl#323 (comment).

I'll play my part and fix wpt-pr-bot to use the review tool as soon as I can find the time to do so.

/cc @jgraham, @sideshowbarker, @RByers, @plehegar, @Ms2ger et al.

@tobie
Copy link
Contributor

tobie commented May 3, 2017

(1) should mostly be fixed now (@wpt-pr-bot just removes the banner except when the PR touches the tools directory).

Will fix (3) next.

The rest isn't in my hands. :)

@domenic
Copy link
Member Author

domenic commented May 3, 2017

Browsing recent PRs, (7) and (4) appear possibly fixed, although I didn't see the commits that did it; maybe those services are just temporarily broken.

@tobie
Copy link
Contributor

tobie commented May 3, 2017

Oh, I still get (7), it's just really slow.

@tobie
Copy link
Contributor

tobie commented May 4, 2017

Fixed (3).

@gsnedders
Copy link
Member

@domenic (7) is a030379; (4) is w3c/prbuildbot@0225fd3

sideshowbarker added a commit to w3c/github_sync that referenced this issue May 5, 2017
Addresses web-platform-tests/wpt#5611

The “These tests are now available on w3c-test.org” from this sync
script has been made redundant by the comments posted from the stability
checker, which also include the link to the mirrored PR branch under
https://w3c-test.org/submissions
@sideshowbarker
Copy link
Contributor

2. A notification that the tests are available on w3c-test.org. (This also appears to get re-posted after every push.)
2 is redundant with 5 and 6, but if people think getting it in a couple minutes before 5 and 6 hit is valuable, then it could edit the original post instead.

With w3c/github_sync@a8218c3 merged and pushed to the webhook location on w3c-test.org, that notification is now gone (completely)

@annevk
Copy link
Member

annevk commented May 5, 2017

Thanks all, great improvements!

@tobie
Copy link
Contributor

tobie commented May 8, 2017

Additionally, it would be great to remove those bot comments when they're not relevant to the changes.

For example, doc changes get the stability bots and code cov comments. That adds a lot of noise and confusion, especially if the contributor is new.

@gsnedders
Copy link
Member

@tobie they shouldn't get code coverage comments since a few weeks ago (thought I think jgraham said we might not get code coverage comments at all now, oops)

@tobie
Copy link
Contributor

tobie commented May 8, 2017

@gsnedders mmm, so I was pretty sure I saw the code coverage comment in a particular pull request, but I might have mixed it up with an older pull request. Apologies.

@bobholt
Copy link
Contributor

bobholt commented May 19, 2017

The remaining points from the original issue are the browser-by-browser test results. I'm putting together a system to consolidate those into a single comment.

@RByers
Copy link
Contributor

RByers commented Aug 16, 2017

@bobholt this is now fully in place, right? Can we call this fixed?

@bobholt
Copy link
Contributor

bobholt commented Aug 16, 2017

@RByers: not quite. There's a bug that appears to be a race condition that causes the new system to occasionally post 3 comments to a PR before it starts updating the original comment. I'm working on that now.

@RByers
Copy link
Contributor

RByers commented Aug 17, 2017

Ok, thanks for the update!

@bobholt
Copy link
Contributor

bobholt commented Oct 4, 2017

This has been resolved.

@bobholt bobholt closed this as completed Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants