Skip to content

jsonwebtoken - Typings for PR413#20321

Closed
Richie765 wants to merge 4 commits intoDefinitelyTyped:masterfrom
Richie765:jsonwebtoken-secret-callback
Closed

jsonwebtoken - Typings for PR413#20321
Richie765 wants to merge 4 commits intoDefinitelyTyped:masterfrom
Richie765:jsonwebtoken-secret-callback

Conversation

@Richie765
Copy link
Copy Markdown
Contributor

See PR auth0/node-jsonwebtoken#413
Please wait until that PR is merged.

@dt-bot
Copy link
Copy Markdown
Member

dt-bot commented Oct 5, 2017

types/jsonwebtoken/index.d.ts

to authors (@SomaticIT @danielheim). Could you review this PR?
👍 or 👎?

Checklist

  • pass the Travis CI test?

@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Unmerged The author did not merge the PR when it was ready. labels Oct 5, 2017
@typescript-bot
Copy link
Copy Markdown
Contributor

This PR has been open and unchanged 5 days without signoff or complaint. This will be merged by a maintainer soon if there are no objections.

(err: Error, encoded: string): void;
}

export interface SecretCallback {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't think this deserves its own type declaration. Can just do:

export type SecretFunc = (header: object, callback: (err: Error, secretOrPrivateKey: string | buffer) => void) => void;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good one 👍. Will update it soon!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Forgot to remove this? It's already been inlined into SecretFunc so should be OK to delete.

@typescript-bot typescript-bot added Unmerged The author did not merge the PR when it was ready. and removed Unmerged The author did not merge the PR when it was ready. labels Oct 19, 2017
@RyanCavanaugh
Copy link
Copy Markdown
Member

This PR has been open and unchanged 5 days without signoff or complaint. This will be merged by a maintainer soon if there are no objections.

@RyanCavanaugh RyanCavanaugh added Unmerged The author did not merge the PR when it was ready. and removed Unmerged The author did not merge the PR when it was ready. labels Nov 1, 2017
@typescript-bot
Copy link
Copy Markdown
Contributor

@Richie765 Please address the merge conflict.

@typescript-bot typescript-bot added Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. and removed Unmerged The author did not merge the PR when it was ready. labels Dec 6, 2017
@minestarks
Copy link
Copy Markdown
Contributor

@Richie765 I'm not sure if you're still waiting on something. Do you still need this PR open?

@minestarks
Copy link
Copy Markdown
Contributor

Closing stale PR for housekeeping reasons. If you would still like these changes to get merged, please open a new PR. Thank you!

@minestarks minestarks closed this Dec 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. Popular package This PR affects a popular package (as counted by NPM download counts). Unmergeable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants