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

Add remove_proxies API for pallet-proxies #7557 #12714

Merged
merged 18 commits into from
May 26, 2023
Merged

Add remove_proxies API for pallet-proxies #7557 #12714

merged 18 commits into from
May 26, 2023

Conversation

vjgaur
Copy link
Contributor

@vjgaur vjgaur commented Nov 16, 2022 •
edited
Loading

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

@vjgaur
Open
4d04516 
Add remove_proxies API for pallet-proxies
paritytech#7557
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Nov 16, 2022

User @vjgaur, please sign the CLA here.

@vjgaur vjgaur mentioned this pull request Nov 29, 2022
Closed
ggwpez
ggwpez previously requested changes Nov 29, 2022
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment •
edited
Loading

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.

frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
@ggwpez ggwpez added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 29, 2022
KiChjang
KiChjang reviewed Dec 14, 2022
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
KiChjang
KiChjang suggested changes Dec 14, 2022
View reviewed changes
Copy link
Contributor

@KiChjang KiChjang left a 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.

@stale
Copy link

stale bot commented Jan 13, 2023

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.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jan 13, 2023
@vjgaur
Copy link
Contributor Author

vjgaur commented Jan 15, 2023

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 .

@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jan 15, 2023
@stale
Copy link

stale bot commented Feb 14, 2023

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.

@stale stale bot added A3-stale and removed A3-stale labels Feb 14, 2023
KiChjang
KiChjang reviewed Feb 15, 2023
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
KiChjang
KiChjang reviewed Feb 15, 2023
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
KiChjang
KiChjang reviewed Feb 21, 2023
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
xlc
xlc reviewed Feb 21, 2023
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
xlc
xlc reviewed Feb 21, 2023
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
@stale stale bot added the A3-stale label May 15, 2023
juangirini
juangirini suggested changes May 23, 2023
View reviewed changes
Copy link
Contributor

@juangirini juangirini left a 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.

frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
@stale stale bot removed A3-stale labels May 23, 2023
@juangirini juangirini 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels May 23, 2023
@vjgaur vjgaur requested a review from a team May 23, 2023 11:48
sam0x17
sam0x17 approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@sam0x17 sam0x17 left a 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 :)

frame/proxy/src/lib.rs Show resolved Hide resolved
juangirini
juangirini reviewed May 23, 2023
View reviewed changes
frame/proxy/src/lib.rs Outdated Show resolved Hide resolved
@juangirini
Copy link
Contributor

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Statuses failed for f1a1f22

@juangirini
Copy link
Contributor

@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?

@juangirini
Copy link
Contributor

bot rebase

@paritytech-processbot
Copy link

Rebased

@liamaharon liamaharon dismissed ggwpez’s stale review May 26, 2023 07:41

changes addressed

@liamaharon
Copy link
Contributor

@juangirini I dismissed @ggwpez's review since his suggestions have been addressed.

You should be able to merge now.

@juangirini
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 46127a9 into paritytech:master May 26, 2023
Ank4n pushed a commit that referenced this pull request Jul 8, 2023
@vjgaur @juangirini @Ank4n
Add remove_proxies API for pallet-proxies #7557 (#12714)
1dba12d 
* 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 <>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@vjgaur @juangirini @nathanwhit
Add remove_proxies API for pallet-proxies paritytech#7557 (paritytech…
24320e8 
…#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 <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xlc xlc xlc left review comments

@sam0x17 sam0x17 sam0x17 approved these changes

@KiChjang KiChjang KiChjang approved these changes

@juangirini juangirini juangirini approved these changes

@liamaharon liamaharon liamaharon approved these changes

@ggwpez ggwpez Awaiting requested review from ggwpez

Assignees

@KiChjang KiChjang

@kianenigma kianenigma

@ggwpez ggwpez

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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add remove_proxies API for pallet-proxies
8 participants
@vjgaur @juangirini @liamaharon @xlc @sam0x17 @KiChjang @ggwpez @kianenigma