⚡ Unify name for Child oriented events - #36
Conversation
| @@ -69,7 +69,7 @@ pub mod rmrk_contract { | |||
| #[ink(event)] | |||
| pub struct ChildAdded { | |||
There was a problem hiding this comment.
It was not changed here but seems a good opportunity to improve. We name this ChildProposed on EVM, because it must be accepted. So "Added" might be confusing.
| pub struct ChildAdded { | ||
| #[ink(topic)] | ||
| to: Id, | ||
| parent: Id, |
There was a problem hiding this comment.
We call this tokenId since the context is the current collection and event is named Child so this token being the parent can be implied. I'd prefer consistency but parent also makes it more explicit so I'm fine with either.
I have mixed thoughts about whether to include "Id" in the names, in general. Here it seems obvious since type is Id, but when calling it, it might not be so obvious. Again, no strong position here, up for discussion.
| parent: Id, | ||
| #[ink(topic)] | ||
| child_collection: AccountId, | ||
| collection: AccountId, |
There was a problem hiding this comment.
I would definitely keep child_collection in the name, here you can't know if this collections is about the parent or the child, both present on the event.
| @@ -93,9 +93,9 @@ pub mod rmrk_contract { | |||
| #[ink(topic)] | |||
There was a problem hiding this comment.
This event is probably legacy. "Removing" a child became "Transferring" a child so event should be ChildTransferred. Since it can happen from pending (rejecting) or active (removing). Maybe we need an issue for this change, I haven't checked nesting lego yet 😞
|
Continue as #38 |
2 participants
For magical reasons there are four events and two of them has different name for collection.