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

Replace simple parameter_types #11254

Merged
paritytech-processbot merged 24 commits into from
Apr 29, 2022
Merged

Replace simple parameter_types #11254

paritytech-processbot merged 24 commits into from
Apr 29, 2022

Conversation

@bizzyvinci
Copy link
Contributor

@bizzyvinci bizzyvinci commented Apr 21, 2022 •
edited
Loading

Fixes #10383 and related to #10402. It's an attempt to replace simple parameter_types declarations with ConstU or ConstBool.

I'm a rust newbie, so I might have screwed up a thing or 2. Also, there are some cases I left out cause I wasn't sure and they include

  1. Should pub static be replaced? pub and pub const were replaced
  2. Some parameter_types declaration were unused. I removed some in the files I edited.
  3. ConstU64<SLOT_DURATION / 2> got to be ConstU64<{ SLOT_DURATION / 2 }>, right?
  4. Some declarations use ::get() method, so they are left as parameter_types
  5. What about ord_parameter_types?
  6. Can ConstantMultiplier<Balance, TransactionByteFee> be replaced with ConstantMultiplier<Balance, ConstU64<0>>
  7. pub const Period and pub const Offset parameter_types were left out. I don't think they are simple

#A0-pleasereview @thiolliere @KiChjang @bkchr

Polkadot address: 1C2pKczetQS7Rpa9m6cyqhsMcV3mpn6RyB68ZR7wBhbBj1N

bizzyvinci added 17 commits April 20, 2022 19:28
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Apr 21, 2022

User @bizzyvinci, please sign the CLA here.

@bizzyvinci bizzyvinci changed the title Parameter types Replace simple parameter_types Apr 21, 2022
kianenigma
kianenigma reviewed Apr 21, 2022
View reviewed changes
kianenigma
kianenigma approved these changes Apr 21, 2022
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

grateful to see all that in fact there were numerous unused types defined as well, and they are now removed.

@shawntabrizi shawntabrizi added 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 labels Apr 23, 2022
@shawntabrizi
Copy link
Member

shawntabrizi commented Apr 23, 2022

Seems you need to fix formatting, and some compiler errors.

I see in the compiler errors there are some places where the variable value is used. In this case, create a const, so that values do not need to be updated multiple places representing the same thing.

@bizzyvinci
Copy link
Contributor Author

bizzyvinci commented Apr 23, 2022

I'll need help to pass the checks, including commits. Don't know if I've been doing things right with the recent commits.

@bkchr
Copy link
Member

bkchr commented Apr 25, 2022

@bizzyvinci the compile errors are not that hard. Just run SKIP_WASM_BUILD=1 cargo check --all --tests and fix all the errors.

@bizzyvinci
Copy link
Contributor Author

bizzyvinci commented Apr 25, 2022

thanks @bkchr. How do I handle format errors?
I ran cargo fmt --check --all and it shows a lot of errors which are not part of my edits.

@KiChjang
Copy link
Contributor

KiChjang commented Apr 25, 2022

@bizzyvinci You need to add +nightly for fmt to work properly, i.e. cargo +nightly fmt

@bizzyvinci
Copy link
Contributor Author

bizzyvinci commented Apr 26, 2022

Thanks @KiChjang. Now, there are 2 more to go.

@KiChjang
Copy link
Contributor

KiChjang commented Apr 29, 2022

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented Apr 29, 2022

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit efe6d77 into paritytech:master Apr 29, 2022
@Kailai-Wang Kailai-Wang mentioned this pull request May 31, 2022
Closed
godcodehunter pushed a commit to sensoriumxr/substrate that referenced this pull request Jun 22, 2022
@bizzyvinci @shawntabrizi
@kianenigma @KiChjang @godcodehunter
Replace simple parameter_types (paritytech#11254)
8fa2147 
* update bin/node-template/runtime/src/lib.rs

* update frame/contracts/src/tests.rs

* update frame/executive/src/lib.rs

* update frame/grandpa/src/mock.rs

* update frame/im-online/src/mock.rs

* update frame/offences/benchmarking/src/mock.rs

* update frame/recovery/src/mock.rs

* update frame/referenda/src/mock.rs

* update frame/session/benchmarking/src/mock.rs

* update frame/staking/src/mock.rs

* update frame/state-trie-migration/src/lib.rs

* update frame/support/test/compile_pass/src/lib.rs

* frame/treasury/src/tests.rs

* update frame/whitelist/src/mock.rs

* update frame/vesting/src/mock.rs

* update test-utils/runtime/src/lib.rs

* update bin/node-template/runtime/src/lib.rs

* Update frame/grandpa/src/mock.rs

Co-authored-by: Kian Paimani <[email protected]>

* resolve failed checks 1518659 & 1518669

* resolve format check

* backtrack to resolve compile error

* check --all --tests ✅

* cargo +nightly fmt ✅

Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Keith Yeung <[email protected]>
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
@bizzyvinci @shawntabrizi
@kianenigma @KiChjang @DaviRain-Su
Replace simple parameter_types (paritytech#11254)
1fdee91 
* update bin/node-template/runtime/src/lib.rs

* update frame/contracts/src/tests.rs

* update frame/executive/src/lib.rs

* update frame/grandpa/src/mock.rs

* update frame/im-online/src/mock.rs

* update frame/offences/benchmarking/src/mock.rs

* update frame/recovery/src/mock.rs

* update frame/referenda/src/mock.rs

* update frame/session/benchmarking/src/mock.rs

* update frame/staking/src/mock.rs

* update frame/state-trie-migration/src/lib.rs

* update frame/support/test/compile_pass/src/lib.rs

* frame/treasury/src/tests.rs

* update frame/whitelist/src/mock.rs

* update frame/vesting/src/mock.rs

* update test-utils/runtime/src/lib.rs

* update bin/node-template/runtime/src/lib.rs

* Update frame/grandpa/src/mock.rs

Co-authored-by: Kian Paimani <[email protected]>

* resolve failed checks 1518659 & 1518669

* resolve format check

* backtrack to resolve compile error

* check --all --tests ✅

* cargo +nightly fmt ✅

Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Keith Yeung <[email protected]>
@bizzyvinci bizzyvinci deleted the parameter_types branch November 12, 2022 18:52
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@bizzyvinci @shawntabrizi
@kianenigma @KiChjang @ark0f
Replace simple parameter_types (paritytech#11254)
77ffdf1 
* update bin/node-template/runtime/src/lib.rs

* update frame/contracts/src/tests.rs

* update frame/executive/src/lib.rs

* update frame/grandpa/src/mock.rs

* update frame/im-online/src/mock.rs

* update frame/offences/benchmarking/src/mock.rs

* update frame/recovery/src/mock.rs

* update frame/referenda/src/mock.rs

* update frame/session/benchmarking/src/mock.rs

* update frame/staking/src/mock.rs

* update frame/state-trie-migration/src/lib.rs

* update frame/support/test/compile_pass/src/lib.rs

* frame/treasury/src/tests.rs

* update frame/whitelist/src/mock.rs

* update frame/vesting/src/mock.rs

* update test-utils/runtime/src/lib.rs

* update bin/node-template/runtime/src/lib.rs

* Update frame/grandpa/src/mock.rs

Co-authored-by: Kian Paimani <[email protected]>

* resolve failed checks 1518659 & 1518669

* resolve format check

* backtrack to resolve compile error

* check --all --tests ✅

* cargo +nightly fmt ✅

Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Keith Yeung <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@athei athei Awaiting requested review from athei athei is a code owner

@andresilva andresilva Awaiting requested review from andresilva

3 more reviewers

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@KiChjang KiChjang KiChjang approved these changes

@kianenigma kianenigma kianenigma approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

Replace parameter_types with ConstU32 &c.

5 participants

@bizzyvinci @shawntabrizi @bkchr @KiChjang @kianenigma