Jackson's Changes for Notifications #1
Conversation
|
Awesome! Thank you @jaxoncreed! |
There was a problem hiding this comment.
It nice that you have merged your version of building well known handling with WellKnownBuilder.
Missing unit tests for WebHookSubscription2021UnsubscribeHttpHandler.
Your comment in src/http/WebHookSubscription2021UnsubscribeHttpHandler.ts line 58 about using the wrong data structure for the notificationData.subscriptions may be valid, but our initial take to use a HashMap were not serializable and could not be put into the storage. Please make a better suggestion.
There was a problem hiding this comment.
Perhaps it would be good to have a link to the notification proposal
https://github.com/solid/notifications-panel/blob/main/proposals/solid-webhook-notifications.md
Into a comment in the classes implementing WebHookSubscription2021.
| callback?: ((res: IncomingMessage) => void) | undefined, | ||
| ): void { | ||
| const parsedUrl = url instanceof URL ? url : new URL(url); | ||
| this.jwksKeyGenerator |
There was a problem hiding this comment.
Perhaps this whole promise-chain could be better implemented using async/await.
TamSzaGot
merged commit 175bf5c
into
individdata:feat/notification-gw-handler-and-webhook
|
Hi @jaxoncreed I think this PR might have been merged/closed prematurely. Did you have unfinished work for this PR? |
|
No, @ixuz. I don't have unfinished work here. Though, it should be noted that the changes in this branch don't have full test coverage. |
|
Understood! Are there any particular reason to keep using |
|
I don't think so. I don't think I actively made the decision to use 3.x.x |
Description
Jackson's Changes to Individdata's notifications