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

jsonwebtoken - Typings for PR413 #20321

Closed

Conversation

Richie765
Copy link
Contributor

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

@dt-bot
Copy link
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 the Popular package This PR affects a popular package (as counted by NPM download counts). label Oct 5, 2017
@typescript-bot typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Oct 10, 2017
@typescript-bot
Copy link
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.

@@ -83,6 +83,14 @@ export interface SignCallback {
(err: Error, encoded: string): void;
}

export interface SecretCallback {
Copy link

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
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

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 removed the Unmerged The author did not merge the PR when it was ready. label Oct 19, 2017
@typescript-bot typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Oct 24, 2017
@RyanCavanaugh
Copy link
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 removed the Unmerged The author did not merge the PR when it was ready. label Nov 1, 2017
@RyanCavanaugh RyanCavanaugh added the Unmerged The author did not merge the PR when it was ready. label Nov 6, 2017
@typescript-bot
Copy link
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
Contributor

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

@minestarks
Copy link
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