Skip to content
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

feat: pallet proxy #70

Merged
merged 9 commits into from
Apr 20, 2024
Merged

feat: pallet proxy #70

merged 9 commits into from
Apr 20, 2024

Conversation

al3mart
Copy link
Collaborator

@al3mart al3mart commented Apr 8, 2024

⚠️ This needs #68 & #69 to be merged first!

Adds pallet-proxy to Pop Network runtimes:

  • devnet
  • testnet

@al3mart al3mart mentioned this pull request Apr 8, 2024
6 tasks
@evilrobot-01
Copy link
Collaborator

Rebased after other two listed PRs merged.

@al3mart al3mart marked this pull request as ready for review April 15, 2024 11:54
@Daanvdplas Daanvdplas self-requested a review April 15, 2024 11:57
@evilrobot-01
Copy link
Collaborator

This PR introduces ~150 lines of duplicate code, signalling future technical debt. Appreciate not all of it can be refactored away due to different runtime types, but surely at least the parameter_types and ProxyType can be moved to common?

Its easy to just duplicate things, but the burden of later maintenance should be considered.

@al3mart
Copy link
Collaborator Author

al3mart commented Apr 17, 2024

I am happy to address this.

@evilrobot-01 based on previous comments my understanding was that most likely we would move away from maintaining different runtimes. Though this should not be a excuse for not having a proper code base today.

Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - pushed one final commit to further eliminate duplicate code.

@evilrobot-01 evilrobot-01 merged commit 5264ace into main Apr 20, 2024
6 checks passed
@evilrobot-01 evilrobot-01 deleted the al3mart/feat-pallet-proxy branch April 20, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants