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

EventType names do not match actual events messages names #79

Closed
1 task done
n0izn0iz opened this issue Jun 14, 2023 · 3 comments · Fixed by #89
Closed
1 task done

EventType names do not match actual events messages names #79

n0izn0iz opened this issue Jun 14, 2023 · 3 comments · Fixed by #89
Assignees
Labels
bug Something isn't working released verified Bug is verified

Comments

@n0izn0iz
Copy link
Contributor

n0izn0iz commented Jun 14, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Package version

v1.10.0

OS

Other

Language version and compiler version

go1.20.4

Bug description

Some event messages names do not match their EventType name
For example the message corresponding to EventType.AccountContactRequestIncomingReceived is actually called AccountContactRequestReceived

This is annoying for writing generic events decoders (it's useful for dumping payloads for example)

I wrote a little script to find inconsistent event naming

❯ npx ts-node packages/scripts/checkWeshnetConsistency.ts
Missing event definition for GroupMemberDeviceAdded
Missing event definition for GroupDeviceChainKeyAdded
Missing event definition for AccountContactRequestOutgoingEnqueued
Missing event definition for AccountContactRequestOutgoingSent
Missing event definition for AccountContactRequestIncomingReceived
Missing event definition for AccountContactRequestIncomingDiscarded
Missing event definition for AccountContactRequestIncomingAccepted
Missing event definition for ContactAliasKeyAdded
Missing event definition for MultiMemberGroupAliasResolverAdded
Missing event definition for MultiMemberGroupInitialMemberAnnounced
Missing event definition for MultiMemberGroupAdminRoleGranted
Missing event definition for GroupMetadataPayloadSent

Current behavior

Some event messages names do not match their EventType name

Expected behavior

All events messages names do match their EventType name

Environment

macOS 13.3.1

Other

Since this is an api breaking change I would recommend fixing this before the api is widely consumed

@n0izn0iz n0izn0iz added the bug Something isn't working label Jun 14, 2023
@jefft0 jefft0 added the verified Bug is verified label Jun 16, 2023
@jefft0 jefft0 self-assigned this Jun 16, 2023
@jefft0
Copy link
Contributor

jefft0 commented Jun 27, 2023

Hello @n0izn0iz . You have a valid point. I agree that we should make breaking API changes now. I have a question. In general the API call is a "verb" and the resulting event is a "noun". For example the API call ContactAddAliasKey causes the event ContactAliasKeyAdded . It seems natural that a command to "add" results in an event which says "added". How do you suggest to rename these? Thanks.

@jefft0 jefft0 added the more info needed Waiting for a response label Jun 27, 2023
@n0izn0iz
Copy link
Contributor Author

n0izn0iz commented Jun 27, 2023

It seems natural that a command to "add" results in an event which says "added". How do you suggest to rename these?

This seems ok, it's not the api call that don't match, it's the event types and messages, in this case, both will be "Added"

I think in the case of ContactAddAliasKey, the api call is ContactAliasKeySend, in the comments, it's stated that ContactAddAliasKey is an event

@jefft0 jefft0 removed the more info needed Waiting for a response label Jun 27, 2023
jefft0 added a commit to jefft0/weshnet that referenced this issue Jun 27, 2023
Signed-off-by: Jeff Thompson <jeff@thefirst.org>
jefft0 added a commit to jefft0/weshnet that referenced this issue Jun 27, 2023
…ent. Fixes berty#79.

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
jefft0 added a commit to jefft0/weshnet that referenced this issue Jun 27, 2023
…ent. Fixes berty#79.

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
@github-actions
Copy link

🎉 This issue has been resolved in version 1.12.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released verified Bug is verified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants