-
Notifications
You must be signed in to change notification settings - Fork 38
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
fix(rest): changed webhook event topic to type #117
fix(rest): changed webhook event topic to type #117
Conversation
af6c14e
to
4fd7321
Compare
Codecov Report
@@ Coverage Diff @@
## main #117 +/- ##
=======================================
Coverage 89.13% 89.13%
=======================================
Files 43 43
Lines 635 635
Branches 91 84 -7
=======================================
Hits 566 566
Misses 59 59
Partials 10 10
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really anything noticeable, but maybe a minor improvement can be made with the agentconfig.
packages/rest/tests/utils/agent.ts
Outdated
@@ -43,8 +43,8 @@ export async function setupAgent({ | |||
], | |||
publicDidSeed: publicDidSeed, | |||
endpoints: endpoints, | |||
autoAcceptConnections: autoAcceptConnection, | |||
autoAcceptCredentials: autoAcceptCredential, | |||
autoAcceptConnections: autoAcceptConnections, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
autoAcceptConnections: autoAcceptConnections, | |
autoAcceptConnections, |
packages/rest/tests/utils/agent.ts
Outdated
autoAcceptConnection: boolean | ||
autoAcceptCredential: AutoAcceptCredential | ||
autoAcceptConnections: boolean | ||
autoAcceptCredentials: AutoAcceptCredential |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just pass the one object that would contain the InitConfig
. Or change some properties to be required with a custom type so that these issues will not occur.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is just used for the sample and testing i have now removed most of the props to make it look cleaner.
packages/rest/package.json
Outdated
"express": "^4.17.1", | ||
"node-fetch": "^2.6.7", | ||
"express": "^4.18.1", | ||
"node-fetch": "2.6.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional to remove the ^
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node-fetch 3.0.0 has some issues, but just learned ^
doesn't update to major version so ill put it back :).
Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
7306f87
to
bf66cba
Compare
Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
…#117) Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
Signed-off-by: Jan 60812202+janrtvld@users.noreply.github.com