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

Deprecation: [@octokit/rest] "number" parameter is deprecated for ".issues.createComment()". Use "issue_number" instead #917

Open
chrishiestand opened this issue Apr 18, 2019 · 10 comments · May be fixed by #926

Comments

@chrishiestand
Copy link
Contributor

commented Apr 18, 2019

Bug Report

Current Behavior
I'm currently getting this deprecation error with probot v9.2.6.

This is the code path:

probotApp.app = function app(probotRouter) {
  probotRouter.on("issue_comment.created", commentAdded);
};

async function commentAdded(context) {
  if (context.payload.sender.type === "Bot") {
    return;
  }

  const issueComment = context.issue({body: "Thanks for adding a comment!"});
  return context.github.issues.createComment(issueComment);
}

Deprecation: [@octokit/rest] "number" parameter is deprecated for ".issues.createComment()". Use "issue_number" instead
at Object.keys.forEach.key (/app/node_modules/probot/node_modules/@octokit/rest/plugins/register-endpoints/register-endpoints.js:73:26)
at Array.forEach ()
at Object.patchedMethod [as createComment] (/app/node_modules/probot/node_modules/@octokit/rest/plugins/register-endpoints/register-endpoints.js:69:26)
at commentAdded (/app/src/probot-app.js:117:32)
at Application. (/app/node_modules/probot/lib/application.js:153:50)
at step (/app/node_modules/probot/lib/application.js:43:23)
at Object.next (/app/node_modules/probot/lib/application.js:24:53)
at fulfilled (/app/node_modules/probot/lib/application.js:15:58)

Expected behavior/code
I expect no deprecation errors

Environment

  • Probot version(s): v9.2.6
  • Node/npm version: node 10.15.3 / npm v6.4.1
  • OS: alpine linux (in docker container node:10-alpine)

Additional context/Screenshots

I'm also having an issue with this code where the bot's issue comment is being created 4 times even though the createComment() method is only called once. But I'm not sure if these two issues are related.

@issue-label-bot issue-label-bot bot added the bug 🐞 label Apr 18, 2019

@issue-label-bot

This comment has been minimized.

Copy link

commented Apr 18, 2019

Issue-Label Bot is automatically applying the label bug 🐞 to this issue, with a confidence of 0.83. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@chrishiestand

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

I'm also seeing this deprecation after the 4 comments are created (I'm thinking maybe after a rate limit error?)

Deprecation: [@octokit/request] error.code is deprecated, use error.status.
at HttpError.get (/app/node_modules/probot/node_modules/@octokit/request/lib/http-error.js:18:17)
at Object.Logger.stdSerializers.err (/app/node_modules/bunyan/lib/bunyan.js:1148:19)
at /app/node_modules/bunyan/lib/bunyan.js:873:50
at Array.forEach ()
at Logger._applySerializers (/app/node_modules/bunyan/lib/bunyan.js:865:35)
at mkRecord (/app/node_modules/bunyan/lib/bunyan.js:978:17)
at Logger.error (/app/node_modules/bunyan/lib/bunyan.js:1044:19)
at Application. (/app/node_modules/probot/lib/application.js:159:33)
at step (/app/node_modules/probot/lib/application.js:43:23)
at Object.throw (/app/node_modules/probot/lib/application.js:24:53)
at rejected (/app/node_modules/probot/lib/application.js:16:65) name: 'Deprecation'

Apparently this was not fixed by: #871

@gr2m

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

regarding the first deprecation, context.issue() needs to be update to set issue_number instead of number. The pull request should also bump the minimal required version of @octokit/rest. Would you like to try sending a pull request to get rid of the deprecation message?

@chrishiestand

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

@gr2m I took a look at the required changes; I don't know typescript so that is a hindrance for me. It seems the pull requests should also change from number to pull_number https://octokit.github.io/rest.js/#octokit-routes-pulls-create-comment
so we should have something like this in context.ts:

interface WebhookPayloadWithRepository {
  [key: string]: any
  repository?: PayloadRepository
  issue?: {
    [key: string]: any
    issue_number: number
    html_url?: string
    body?: string
  }
  pull_request?: {
    [key: string]: any
    pull_number: number
    html_url?: string
    body?: string
  }

Then we change the object key to the appropriate key in the method below. But I'm not sure what to do with the third term || payload:

  public issue<T> (object?: T) {
    const payload = this.payload
    return Object.assign({
      number: (payload.issue || payload.pull_request || payload).number
    }, this.repo(object))
  }

When payload does not have an issue key, or a pull_request key, what is it?

@swinton

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

🚧 Fix in #926

@mrchief

This comment has been minimized.

Copy link

commented May 2, 2019

Apparently this was not fixed by: #871

I second that. I'm seeing this in v9.2.9. I commented on a closed issue but maybe this is a better place?

@stale

This comment has been minimized.

Copy link

commented Jul 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 1, 2019

@mrchief

This comment has been minimized.

Copy link

commented Jul 1, 2019

Still an issue

@stale stale bot removed the wontfix label Jul 1, 2019

@gr2m gr2m added the pinned label Jul 1, 2019

@praWeb

This comment has been minimized.

Copy link

commented Jul 5, 2019

It is still happening, I am not able to use my app which was working before.

@gr2m

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2019

This is just a deprecation warning, not an Error. It should not affect your app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.