-
Notifications
You must be signed in to change notification settings - Fork 352
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
Add field names to pallet Event variants #1277
Conversation
@warfollowsme Thank you for your contribution, it looks good. |
@crystalin Do I need fixed all diffs from the build log too? |
No need to fix them manually, you just need to run |
okay. done. :) |
Okay. I realized that I completely forgot to change the use of events in tests. |
|
You probably just need to merge master |
Okay. Only docker compile left. Issues with login. How can I fix it? |
You can't fix it, we need to skip theses jobs for external PRs. @crystalin can you merge this PR ? |
@librelois if this is inconsistent with our other pallets, I'd prefer to have them all included the same format before merging it (to avoid having multiple breaking changes release) what do you think ? |
Actually, does it impact the clients (polkadotJS UI or SDK) ? It doesn't seem so looking at the file changed. if not I'll merge it |
We have a lot of events with unnamed fields (basically everywhere…), I think it would be a too big PR to change everything at once, it's better to do it in several times :) |
I know that polkadotjs is not impacted, for the rest I don't know |
Merging, thank you @warfollowsme for your contribution |
Description
This PR changes/added:
After PR#9896 was merged events can now be described with field names.
These field names and their respective comments are in turn reflected in the metadata for a better client experience.