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

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.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@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

@shawntabrizi shawntabrizi requested a review from sam0x17 August 18, 2022 16:49
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.

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
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…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.
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
Archived in project
Development

Successfully merging this pull request may close these issues.

construct_runtime to allow for cfg macro for pallets
5 participants