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

fix(rest): changed webhook event topic to type #117

Merged

Conversation

janrtvld
Copy link
Contributor

@janrtvld janrtvld commented Jun 8, 2022

  • Fixed webhook event
  • Updated dependencies
  • Typo

Signed-off-by: Jan 60812202+janrtvld@users.noreply.github.com

@janrtvld janrtvld requested a review from a team as a code owner June 8, 2022 13:27
@codecov-commenter
Copy link

codecov-commenter commented Jun 8, 2022

Codecov Report

Merging #117 (4fd7321) into main (9900423) will not change coverage.
The diff coverage is 0.00%.

@@           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           
Impacted Files Coverage Δ
packages/rest/src/events/WebhookEvent.ts 80.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9900423...4fd7321. Read the comment docs.

Copy link
Contributor

@berendsliedrecht berendsliedrecht left a 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.

@@ -43,8 +43,8 @@ export async function setupAgent({
],
publicDidSeed: publicDidSeed,
endpoints: endpoints,
autoAcceptConnections: autoAcceptConnection,
autoAcceptCredentials: autoAcceptCredential,
autoAcceptConnections: autoAcceptConnections,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
autoAcceptConnections: autoAcceptConnections,
autoAcceptConnections,

autoAcceptConnection: boolean
autoAcceptCredential: AutoAcceptCredential
autoAcceptConnections: boolean
autoAcceptCredentials: AutoAcceptCredential
Copy link
Contributor

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.

Copy link
Contributor Author

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.

"express": "^4.17.1",
"node-fetch": "^2.6.7",
"express": "^4.18.1",
"node-fetch": "2.6.7",
Copy link
Contributor

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

Copy link
Contributor Author

@janrtvld janrtvld Jun 8, 2022

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>
Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
@TimoGlastra TimoGlastra merged commit fed645e into openwallet-foundation:main Jun 10, 2022
janrtvld added a commit to janrtvld/aries-framework-javascript-ext that referenced this pull request Aug 26, 2022
…#117)

Signed-off-by: Jan <60812202+janrtvld@users.noreply.github.com>
Sign up for free to 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.

None yet

4 participants