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

Notifications serialise connect, disconnect, block_added #6345

Closed
wants to merge 5 commits into from
Closed

Notifications serialise connect, disconnect, block_added #6345

wants to merge 5 commits into from

Conversation

ShahanaFarooqui
Copy link
Collaborator

@ShahanaFarooqui ShahanaFarooqui commented Jun 18, 2023

  • Serialise connect, discnnect and block_added.

Changelog-Added: Plugins: connect, disconnect, block_added notifications now have fields in sub-object of same name (e.g. "connect": {...}), making all notifications uniform
Changelog-Deprecated: Plugins: connect, disconnect, block_added notifications fields outside the sub-object of the same name: e.g. parse .connect.id instead of .id.

lightningd/notification.c Outdated Show resolved Hide resolved
@@ -87,9 +87,11 @@ A notification for topic `connect` is sent every time a new connection to a peer

```json
{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should talk about the form of all these notifications (i.e. that notification X will contain an object X) in the general section of this doc.

Should we remove PLUGINS.md too, as a previous commit? It's confusing to have both!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree, it is redundant but we can leave it till I am not working on documentation consolidation. I will work on the logical placement of all these documents at that point anyways.

@ShahanaFarooqui ShahanaFarooqui marked this pull request as ready for review June 22, 2023 03:56
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Changelog-Added: Plugins: connect, disconnect, block_added notifications now have fields in sub-object of same name (e.g. "connect": {...}), making all notifications uniform
Changelog-Deprecated: Plugins: connect, disconnect, block_added notifications fields outside the sub-object of the same name: e.g. parse .connect.id instead of .id.

@ShahanaFarooqui ShahanaFarooqui changed the title Notifications serialise connect and disconnect Notifications serialise connect, disconnect, block_added Jun 28, 2023
tests: notification response fixes
@ShahanaFarooqui ShahanaFarooqui requested a review from cdecker as a code owner June 29, 2023 17:30
@ShahanaFarooqui ShahanaFarooqui closed this by deleting the head repository Jul 4, 2023
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.

2 participants