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

Replace parameter_types with ConstU32 &c. #10383

Closed
gavofyork opened this issue Nov 28, 2021 · 12 comments · Fixed by #11254
Closed

Replace parameter_types with ConstU32 &c. #10383

gavofyork opened this issue Nov 28, 2021 · 12 comments · Fixed by #11254
Labels
I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder

Comments

@gavofyork
Copy link
Member

gavofyork commented Nov 28, 2021 •
edited
Loading

Most simple parameter_types declarations can be replaced with the new ConstU32, ConstU64, ConstU128 and ConstBool, reducing some unneeded verbosity.
@gavofyork gavofyork added I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder labels Nov 28, 2021
@dharjeezy
Copy link
Contributor

Picking this up @gavofyork

@dharjeezy
Copy link
Contributor

dharjeezy commented Nov 28, 2021 •
edited
Loading

@gavofyork i see in your PR here #10382 that you have defined the implementation for ConstU64, ConstU128 and ConstBool. Only ConstU32 is available currently in master.

Should i wait for your PR to get merged before solving this issue?

@gavofyork
Copy link
Member Author

gavofyork commented Nov 28, 2021 •
edited
Loading

You could just start on a branch which brings in that change. It's not a lot of code.

@ferrell-code ferrell-code mentioned this issue Nov 29, 2021
Closed
@dharjeezy
Copy link
Contributor

When you say start on a branch, Are you saying i should start off on my own branch which I already started(changing parameter types for ConstU32)?

More so, are you saying i should implement the ConstU64, ConstU128 and ConstBool on my own branch?

@gavofyork

@gavofyork
Copy link
Member Author

gavofyork commented Dec 1, 2021 •
edited
Loading

More so, are you saying i should implement the ConstU64, ConstU128 and ConstBool on my own branch?

Yes, you could copy that code in if it's going to be useful (and it probably is).

@dharjeezy dharjeezy mentioned this issue Dec 2, 2021
Merged
@dharjeezy
Copy link
Contributor

I have started the implementation as opened in the PR
@gavofyork

@KiChjang
Copy link
Contributor

Fixed by #10402.

@gui1117
Copy link
Contributor

gui1117 commented Dec 20, 2021

some place could still be replaced #10402 (comment)

@gui1117 gui1117 reopened this Dec 20, 2021
@bizzyvinci
Copy link
Contributor

bizzyvinci commented Apr 17, 2022 •
edited
Loading

This issue is still open? @thiolliere

@bkchr
Copy link
Member

bkchr commented Apr 19, 2022

@bizzyvinci have you checked the mentioned code?

@bizzyvinci
Copy link
Contributor

bizzyvinci commented Apr 20, 2022 •
edited
Loading

Yes.
This seems resolved already but the issue remains open

@bizzyvinci
Copy link
Contributor

@bkchr I've searched through the code repo and it appears there are places that uses parameter_types. I'll see which one I can take up and make a PR. I'm new in rust so I'll use the merged pr as a guide.

@bizzyvinci bizzyvinci mentioned this issue Apr 21, 2022
Merged
@wilwade wilwade mentioned this issue Apr 26, 2023
Merged
wilwade added a commit to frequency-chain/frequency that referenced this issue May 2, 2023
@wilwade
Decrease use of parameter types (#1453)
e0688af 
# Goal
The goal of this PR is to reduce and remove code

Split off of #1448 

# Discussion
- Turns out that parameter_types! is not needed as much anymore so
removed a bunch of them (based on
paritytech/substrate#10383 )
- Removed the need for custom implementations on
`MaxSchemaGrantsPerDelegation`
- Aligned the BlockNumber (old u64) in testing with the BlockNumber
(u32) in production
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace simple parameter_types bizzyvinci/substrate
6 participants
@gavofyork @KiChjang @bkchr @gui1117 @dharjeezy @bizzyvinci