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

Request: don't comment on error #238

Closed
cjihrig opened this issue Jun 6, 2019 · 9 comments
Closed

Request: don't comment on error #238

cjihrig opened this issue Jun 6, 2019 · 9 comments

Comments

@cjihrig
Copy link

cjihrig commented Jun 6, 2019

Sadly, an error occurred when I tried to trigger a build. :(

These comments don't really add value to GitHub threads, create additional notification spam, and are likely confusing for newcomers. Is it possible for the bot to just not comment, and instead email (or otherwise notify) bot maintainers when errors occur.

@phillipj
Copy link
Member

phillipj commented Jun 6, 2019

I see your point indeed. Too busy at work to fix it ASAP, but anyone feel free to only invoke createPrComment() on successes about here: ./scripts/trigger-jenkins-build.js#L97.

github-bot at nodejs.org might be suitable?

@refack
Copy link
Contributor

refack commented Jun 6, 2019

We could make these into a Status API call, which is less intrusive...

@phillipj
Copy link
Member

phillipj commented Jun 8, 2019

True @refack. As a bot maintainer I appreciated getting noticed by collaborators when something starts to fail, but getting an email every time will be quite annoying. Guess that annoyance is in practise what feature request is all about, just with PR comments rather than emails.

Posting a failed status via the Status API like we do for Jenkins job statuses today, would bring the point across and allow collaborators to open an issue in this repo or contact bot maintainers somehow.

On second thought, only sending that email, wouldn't let the collaborators know there's currently some issues kicking off Jenkins builds. It would rather be that nothing happens, which I assume would cause more confusion and uncertainty than getting some kind of feedback about the failure from the bot's point of view.

@phillipj
Copy link
Member

For ref now that #228 just rolled out, the bot should start re-using / edit its previous PR comment, which I'm assuming will also help a lot to reduce the previous comment noise it has generated.

@richardlau
Copy link
Member

richardlau commented Jun 11, 2019

For ref now that #228 just rolled out, the bot should start re-using / edit its previous PR comment, which I'm assuming will also help a lot to reduce the previous comment noise it has generated.

I kicked off https://ci.nodejs.org/job/node-test-pull-request/23781/ for nodejs/node#28128 but the bot doesn't appear to have updated any of the comments it previously made in that PR.

@phillipj
Copy link
Member

Thanks @richardlau! Found this in the logs:

00:42:03.038Z ERROR bot: Error while creating comment on GitHub (req_id=baae5340-8be1-11e9-a4d0-856e0a231c31)
    GraphqlError: Variable number of type Int! was provided invalid value
        at request.then.response (/home/../node_modules/@octokit/graphql/lib/graphql.js:31:15)

I'll dive into this to see what might be causing it..

@phillipj
Copy link
Member

Rolled a couple of improvements (2488a2b, 98f7322) which should either fix the issue or give more detailed info about what/when failed.

@richardlau
Copy link
Member

Started https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3673/ for nodejs/node#28178. No edit/update in the issue from the bot.

phillipj added a commit to phillipj/github-bot that referenced this issue Jun 15, 2019
As the bot is does not have permission to trigger Jenkins Lite-CI builds
for some reason at the moment, and there's feedback about these error
comments not being useful to collaborators anyways, those comments are
muted for now.

Ideally github-bot maintainers should get notified about these errors,
as they're obviously not meant to be raised at all. But using
node core collaborators as proxy, manually telling bot maintainers about
errors surely isn't idealy either.

Refs nodejs#238
Refs nodejs#235 (comment)
phillipj added a commit that referenced this issue Jun 17, 2019
As the bot is does not have permission to trigger Jenkins Lite-CI builds
for some reason at the moment, and there's feedback about these error
comments not being useful to collaborators anyways, those comments are
muted for now.

Ideally github-bot maintainers should get notified about these errors,
as they're obviously not meant to be raised at all. But using
node core collaborators as proxy, manually telling bot maintainers about
errors surely isn't idealy either.

Refs #238
Refs #235 (comment)
@phillipj
Copy link
Member

Soooo #241 rolled out today, which removed the error comments completely for now.

That's still not ideal as bot maintainers really do need a hint about these kinds of failures when they start happening. Building upon #238 (comment) about using the Status API to report these failures, is that something that potentially could break node-core-utils @joyeecheung? Or isn't statuses used there at all?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants