-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add remove_proxies API for pallet-proxies #7557 #12714
Conversation
Add remove_proxies API for pallet-proxies paritytech#7557
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.
I think we probably need a new trait or whatever that this pallet implements. Then other pallets can call into it.
Please ensure that at least the tests work before you submit code here. cargo test -p pallet-proxy
at minimum, thanks.
In progress issue number 7557, merged with latest upstream
Merged latest upstream with issue 7557 in progress
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.
This PR still has not done what the issue is asking for.
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
I will be working on it . |
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
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
Just a nit that would make the new method more self explanatory.
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.
Looks good to me if you add a quick rust doc comment on the new method :)
bot merge |
Waiting for commit status. |
Merge cancelled due to error. Error: Statuses failed for f1a1f22 |
@ggwpez I have called "bot merge" but didn't realise you had to approve this too as there is a pending request from you. Could you please have a look at it? |
bot rebase |
Rebased |
@juangirini I dismissed @ggwpez's review since his suggestions have been addressed. You should be able to merge now. |
bot merge |
* Open Add remove_proxies API for pallet-proxies #7557 * added remove_all_proxy_delegates method * remove all proxy implementation * remove_all_proxy_delegate * reverted changes * fixed 7557 * fixed warnings * removed println! causing build failure on ci/cd pipelines * incorporated suggested changes * minor change * made suggested changes * method comment * Update frame/proxy/src/lib.rs --------- Co-authored-by: Juan <juangirini@gmail.com> Co-authored-by: parity-processbot <>
…#12714) * Open Add remove_proxies API for pallet-proxies paritytech#7557 * added remove_all_proxy_delegates method * remove all proxy implementation * remove_all_proxy_delegate * reverted changes * fixed 7557 * fixed warnings * removed println! causing build failure on ci/cd pipelines * incorporated suggested changes * minor change * made suggested changes * method comment * Update frame/proxy/src/lib.rs --------- Co-authored-by: Juan <juangirini@gmail.com> Co-authored-by: parity-processbot <>
Added a new method pub remove_all_proxy_delegates, moved the logic of remove_proxy there, and calling this remove_all_proxy_delegates from remove_proxies.
Fixes #7557