-
Notifications
You must be signed in to change notification settings - Fork 116
Add replace_lead() to content working group
#252
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 replace_lead() to content working group
#252
Conversation
replace_lead() to content working group
|
|
||
| // Update next lead counter | ||
| <NextLeadId<T>>::mutate(|id| *id += <LeadId<T> as One>::one()); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, assert() can be replaced with "?" function invocation. Code reordering is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good point, I think this is the new convention we decided on. However here I avoided changing the implementation of set_lead and unset_lead I just relocated them and converted them from being dispatchables to be private methods on the Module.
I can update it not much extra work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in e4f57a2
|
|
||
| let unregistered_role = <members::Module<T>>::unregister_role(current_lead_role).is_ok(); | ||
|
|
||
| assert!(unregistered_role); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, assert() can be replaced with "?" function invocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in e4f57a2
…te-runtime-joystream into content-wg-replace-lead
also drop call to can_register_role_on_member() it is called by register_role_on_member()
shamil-gadelshin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Implements #243