Skip to content

Conversation

@refack
Copy link
Contributor

  • Add @octokit/graphql

constgetPRComments=`query getPRComments($owner: String!, $repo: String!, $number: Int!, $cursor: String){
repository(owner: $owner, name: $repo){
pullRequest(number: $number){
comments(first: 20, after:$cursor){
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
comments(first: 20,after:$cursor){
comments(first: 100,after:$cursor){

I just put 20 to see if the recursive querying works.

constnewBody=`${oldBody}\n${body}`
returngithubClient.issues.editComment({ owner, repo, id,body: newBody})
}
returngithubClient.issues.createComment({ owner, repo, number, body })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure like the looks of using GraphQL and promises 👍

nodes{
name
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Planning to use these labels for something?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to recognize a bot-out label, for complete opt-out.

@phillipj
Copy link
Member

Leaning towards keeping these changes instead of what I suggested in #226, especially when it comes to talking to the GitHub API.

The other difference from the mentioned PR, is tests. You got any plans for more tests?

@refack
Copy link
ContributorAuthor

You got any plans for more tests?

That's my current WIP... mocking the GQL calls. (H/T to @gr2m for https://github.com/octokit/graphql.js/#writing-tests)

@gr2m
Copy link

gr2m commented Mar 28, 2019

Holler if you run into any blockers or have any questions about GitHub APIs / Octokit, happy to help

@phillipj
Copy link
Member

Any updates on this? I fully understand if you've got other things that needs attention -- if so, maybe we should consider merging #226 at first to reduce the amount of CI comment noise that @cjihrig was concerned about? 🤷‍♂️ ..then merge this as an improvement as soon as it's ready.

@phillipj
Copy link
Member

@refack would you mind if I dived into getting a test in place, with the goal of getting this merged & deployed?

@phillipjphillipjforce-pushed the refine-pr-comment branch from 7212854 to 9e6f42cCompareMay 5, 2019 20:07
@phillipj
Copy link
Member

@refack as I would really appreciate getting this landed soon, I took the freedom of rebasing with master to fix a package-lock.json merge conflict and pushed a couple of extra commits with missing GraphQL fixtures & tests -- sorry in advance if I crossed a line by doing that.

PTAL

@phillipj
Copy link
Member

@refack@cjihrig is this still something we want rolled out or are you happy with how things work currently?

@cjihrig
Copy link

I'd still prefer if the bot limited its commenting when possible.

@phillipj
Copy link
Member

Alrighty then, let's give this a test round to see how it behaves 🚀

Rebased and force push to fix conflict in package-lock.json.

@phillipjphillipj merged commit 1cbae04 into nodejs:masterJun 10, 2019
@phillipj
Copy link
Member

This is now live in production.

phillipj added a commit to phillipj/github-bot that referenced this pull request Jun 15, 2019
Reverting changes related to editing existing comments, going back to the previous behaviour of always creating new comments in PRs about CI / Jenkins runs. This is done primarily due to two things: * It has broken node-core-utils' ability to check if CI's have been run * There's not collaborator consensus about editing comments is actually better than creating new comments Future plans for bot PR comments going forward has to be discussed more thorougly and ensured it doesn't break other automation tools used by collaborators. Refs nodejs#228
phillipj added a commit that referenced this pull request Jun 15, 2019
Reverting changes related to editing existing comments, going back to the previous behaviour of always creating new comments in PRs about CI / Jenkins runs. This is done primarily due to two things: * It has broken node-core-utils' ability to check if CI's have been run * There's not collaborator consensus about editing comments is actually better than creating new comments Future plans for bot PR comments going forward has to be discussed more thoroughly and ensured it doesn't break other automation tools used by collaborators. Refs #228
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@refack@phillipj@gr2m@cjihrig