Improve Developer Experience + Understanding Code of Pallet and Runtime Events
#[pallet::config]
pub trait Config: frame_system::Config {
/// Because this pallet emits events, it depends on the runtime's definition of an event.
type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
...
}
We know there is an "outer Event" being generated in the runtime, and there is also a local "pallet Event" being defined in each pallet. There is also a "system Event"...
The fact that all of these named Event is not very great, and can make it hard to understand what the line does.
I propose we can be more specific and rename things to:
#[pallet::config]
pub trait Config: frame_system::Config {
/// Because this pallet emits events, it depends on the runtime's definition of an event.
type RuntimeEvent: From<PalletEvent<Self>> + IsType<<Self as frame_system::Config>::SystemEvent>;
...
}
Where we use RuntimeEvent, PalletEvent, and SystemEvent to make it more clear what is going on.
Not really fixed on these specific names, but I think that this direction is better, and the extra characters here are worth it.
Picking this up. @shawntabrizi
@dharjeezy I havent had a chance to talk to the others in Substrate development about this change, so let's not jump on it so quickly.
If you need some good projects, I recommend looking at some of the open issues in https://github.com/paritytech/bench-bot/issues or even https://github.com/paritytech/substrate-tip-bot
Alright @shawntabrizi Thanks for letting me understand, I am looking at contributing to rust projects basically. I will appreciate if you let me know when this issue is available for pick up after confirming from the substrate dev team. Thanks once again.
So, in line with the spirit discussed in #11736, I think instead of calling it OuterEvent, we should instead call it RuntimeEvent.
SystemEvent and PalletEvent looks fine to me.
@KiChjang 100%, updated everything to represent this
Sorry for silly question but in frame_system pallet
2 /// The aggregated event type of the runtime.
1 type RuntimeEvent: Parameter
299 + Member
1 + From<Event<Self>>
2 + Debug
3 + IsType<<Self as frame_system::Config>::RuntimeEvent>;
IsType<> Bound is redundant ?
Sorry for silly question but in frame_system pallet
2 /// The aggregated event type of the runtime. 1 type RuntimeEvent: Parameter 299 + Member 1 + From<Event<Self>> 2 + Debug 3 + IsType<<Self as frame_system::Config>::RuntimeEvent>;IsType<> Bound is redundant ?
As far as I can tell it makes sure that all the RuntimeEvent(s) always inherit from the ‘frame_system’ RuntimeEvent. Which allows access to whatever methods that has.
That I understand but it seems redundant for system pallet itself. It makes sense for all other pallets
You can always just delete it and see if everything still works.
You can always just delete it and see if everything still works.
I have tried it but it gives compilation error from parse of macro expansion. It is because the type name is Runtime event, perhaps it will work if I rename it to Systemevent. I will try PR soon.
Created WIP PR https://github.com/paritytech/substrate/pull/13541 To demonstrate things works fine I have renamed all references of system::Config::RuntimeEvent to SystemEvent in pallet assets. Similar work is required for all other pallet. If we really want this change in substrate and other companion project I can take my PR forward. :pray: @shawntabrizi
type RuntimeEvent: From<PalletEvent<Self>> + IsType<<Self as frame_system::Config>::SystemEvent>;
This must have been a mistake because frame_system::Config::Event (RuntimeEvent) is also the event type as declared in the runtime.
The issue here was also already done by renaming Event to RuntimeEvent. Sorry for the "wasted" work @vivekvpandya