Bot does not post or edit comment on new Jenkins builds

[phillipj]

After #228 got merged and rolled out, the bot has not been able to post (or edit) comments to related PRs. Creating this task to track progress and discussions surrounding that issue, rather than using #238 for that matter.

---

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.

Originally posted by @richardlau in #238 (comment)

[phillipj]

Latest update on logs:

As a result of merging #228 these errors kinds of errors was logged:

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)

After yesterdays tweaks (2488a2b, 98f7322), no errors gets logged anymore. Though as @richardlau points out, there's still no comments being created or edited by the bot when new Jenkins builds are started.

Throwing a couple of thoughts out there:

1. As no comments get affected, the code creating / editing comment doesn't get invoked for some reason?
2. The test(s) created in Try to find a comment to edit before creating #228 is wrong or lacking since we're not able to catch what's wrong here without testing in production

[richardlau]

So I have now seen this work, e.g. nodejs/node#28179:

Sadly, an error occurred when I tried to trigger a build. :(
CI: https://ci.nodejs.org/job/node-test-pull-request/23809/
CI: https://ci.nodejs.org/job/node-test-pull-request/23816/

[phillipj]

Oh that's great! Let's keep this issue open a day or two anyways just in case there's some weirdness going on afterall.

[sam-github]

You all are working hard on this, so I hate to be critical, but ... sorry, I'm seeing this working now, nodejs/node#28211 (comment) and I wish it wasn't. I'm not finding it as useful as the previous behaviour.

By not emitting the ci messages into the commit stream the ordering relationship is now gone, I can't see the series of rebases, publishes, etc. with the ci runs interspersed. I also have to scroll up to the top of the page to see the ci links, and there are just a list of links with no idea when they happened, what commit they ran on. I know I can click through and stare at jenkins to get that info, but it used to be visible in github.

Slightly unrelated, but that any time there is an internal CI auto-trigger failure I get a message in the event stream isn't really useful to me. It suceeds so seldomly, that I don't even remember what kind of build was going to get auto-triggered, but I think its a ci-lite, isn't it? So isn't this a fragile duplication of what travis is already doing?

I think since its an internal github bot failure, the notications should go to the bot maintainers, not the comment stream. (EDIT: or not be done at all)

In theory, the message could be useful to me, because if ci-lite didn't get kicked off, then I have to kick off a ci manually... except that most PRs require a full-ci, so I already have to kick off a CI manually. Right now, these failed messages are just an extra email I get and delete every time I open a PR.

Which brings me full circle to this feature to use commit message edit to reduce email. I go through a lot of github notifications thrown at me via email, and ones that simply say ci: ... aren't even near to the top of the ones I wish I could auto-filter out, because they are a small percentage, and they require no thought to delete. They are small and easy to scan and identify, and easy to dismiss as uninteresting (if I don't want to know).

Was collaborator-wide feedback on this feature gathered? There are going to be some various points of view, but it may be that more people are happy about the old ci messages than unhappy.

Note that external contributors used to get a message when a collaborator triggered a full-ci for them, which I assume would be useful, because they know its going on and can go check its results.

Now when I trigger a full-ci for some non-collab PR, they won't even know it happened, or know to go look at the result, so I'll have to either add a message telling them (so we are back to another email notifcation, except I had to write it manually), or I'll have to monitor the PR myself and ping them if it failed, which is excessively time consuming.

As for the suggestion of making the auto-ci show when it fails to start as a failure in the github ci API results sense, down in the build results, I don't know exactly how that works, but it sounds like it might have not-useful interaction with the full-ci which might have gotten manually started, and which should override any auto-ci.

[richardlau]

I've speculated in Slack and @refack confirmed that the change to edit comments rather than post new ones has broken node-core-utils' ability to check if CI's have been run (due to not being able to correctly place them now timewise in relation to commits being pushed).

I've also seen @Trott and @joyeecheung manually post comments with the same CI URL's that are edited into the github-bot's previous comment -- whether that's because they don't realise the bot has made the edit only they can answer.

[sam-github]

OK, then whether the editing stays in or out, can it be reverted in the short term? Beaking git node land is not good.

[joyeecheung]

I'd prefer the bot just post a new message, TBH. Imagine if the first CI is launched in the middle of a very long code review, which gets folded by GitHub, the next time someone wants to check the lastest CI they would have to find the reply in the thread first.

[phillipj]

You all are working hard on this, so I hate to be critical, but ... sorry

No need to be sorry, personally very much appreciate feedback, thank you. I don't have strong opinions on editing or posting new comments, the most important thing is that it's useful to collaborators.

.. whether the editing stays in or out, can it be reverted in the short term?

Absolutely, I'll do that right away. Also sounds like there's consensus muting error comments for now, especially since there's some Jenkins permissions issue causing that to fail consequently these days (#235 (comment)).

Would be very helpful getting some thoughts on the best approach to gathering insights from collaborators on this topic. How would you do that? E.g. open an issue in the node repo?

[phillipj]

Opened #240 and #241 to fix the above for now.

[phillipj]

Closing this issue for now as editing has been reverted and new CI comments are created successfully.