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

Allow construct_runtime to take cfg attributes for pallets #11818

Merged
merged 15 commits into from
Aug 25, 2022
Merged

Allow construct_runtime to take cfg attributes for pallets #11818

merged 15 commits into from
Aug 25, 2022

Conversation

KiChjang
Copy link
Contributor

Fixes #10286.

This PR allows #[cfg] attributes to exist on top of pallet declarations in the construct_runtime! macro, and it works exactly in the way that you would intuitively expect -- the pallet immediately following the attribute would exist in the runtime only if the feature gate as described by the cfg attribute is enabled, otherwise it does not exist.

@KiChjang KiChjang added A0-please_review Pull request needs code review. B3-apinoteworthy 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 Jul 12, 2022
@kianenigma
Copy link
Contributor

wen #cfg(test) dispatchable? (#11754)

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1745298

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1745299

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable-int
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1745310

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1745339

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1745340

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750310

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750312

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750387

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750388

@shawntabrizi
Copy link
Member

cc @sam0x17

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750445

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750446

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750447

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750510

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750509

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750511

sam0x17
sam0x17 approved these changes Aug 18, 2022
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.

This makes sense to me and is a very reasonable improvement. I found some missing end-of-file newlines on a few files that would be cool to fix but other than that this looks good--macro code is well structured.

frame/support/test/tests/construct_runtime_ui/unsupported_pallet_attr.rs Outdated Show resolved Hide resolved
frame/support/test/tests/construct_runtime_ui/unsupported_meta_structure.rs Outdated Show resolved Hide resolved
frame/support/test/tests/construct_runtime_ui/invalid_meta_literal.rs Outdated Show resolved Hide resolved
shawntabrizi
shawntabrizi approved these changes Aug 23, 2022
View reviewed changes
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

high level, non-expert macro review

@KiChjang
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 1c2cdfc into master Aug 25, 2022
@paritytech-processbot paritytech-processbot bot deleted the kckyeung/construct-runtime-cfg-attr branch August 25, 2022 08:56
@librelois librelois mentioned this pull request Sep 15, 2022
Closed
2 tasks
@KiChjang KiChjang mentioned this pull request Sep 27, 2022
Merged
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@KiChjang @ark0f
Allow construct_runtime to take cfg attributes for pallets (paritytec…
fbd1624 
…h#11818)

* Allow construct_runtime to take cfg attributes for pallets

* cargo fmt

* Remove commented out code

* Fixes

* cargo fmt

* Remove inaccurate comments

* Fix typo

* Properly reverse pallets

* Fixes

* cargo fmt

* Add missing newlines
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sam0x17 sam0x17 sam0x17 approved these changes

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@kianenigma kianenigma Awaiting requested review from kianenigma

@bkchr bkchr Awaiting requested review from bkchr

@ggwpez ggwpez Awaiting requested review from ggwpez

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. 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
Runtime / FRAME
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

construct_runtime to allow for cfg macro for pallets
5 participants
@KiChjang @kianenigma @paritytech-cicd-pr @shawntabrizi @sam0x17