Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Migrate xcm pallet to Named Events #5461

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zqhxuyuan
Copy link
Contributor

follow up: #5422 on pallet-xcm

@gilescope
Copy link
Contributor

This seems like a good idea to me as people can match on bits they are interested and .. the rest. That way if we add some field later there's potentially less code to change by the user of the event.

@shawntabrizi
Copy link
Member

@zqhxuyuan good progress! you are doing everything right. Just need to address the compiler errors, which is probably just migrating more event syntax to the new format

@zqhxuyuan
Copy link
Contributor Author

done.

///
/// \[ id, response \]
ResponseReady(QueryId, Response),
ResponseReady { id: QueryId, response: Response },
Copy link
Contributor

Choose a reason for hiding this comment

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

There's loads of ids in this world. I think it's really helpful to prefix it with the type of id it is. In this case query_id. (Shawn has done this in the prior PR for an index field and I think that's a good shout. There's lot's of indices too!)

@gilescope gilescope added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels May 14, 2022
@bkchr
Copy link
Member

bkchr commented May 14, 2022

No xcm changes outside the xcmv3 branch. If you want this to be merged, please change the target branch to xcmv3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants