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

Emit error when Config part is imported but without the std feature #9225

Merged
13 commits merged into from
Jul 16, 2021

Conversation

KiChjang
Copy link
Contributor

This PR emits a compiler error when construct_runtime detects that the Config part is imported, but the corresponding pallet crate does not have the std feature enabled, which would cause the PalletConfig struct to be undefined.

@KiChjang KiChjang requested a review from gui1117 June 29, 2021 04:52
@KiChjang KiChjang added A0-please_review Pull request needs code review. 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 Jun 29, 2021
@KiChjang
Copy link
Contributor Author

I've no idea how to test for this, since it requires import from a test fixtures crate.

@gui1117
Copy link
Contributor

gui1117 commented Jun 29, 2021

for test we can have 1 test pallet in frame/support/test/pallet and we import it without std in frame/support/test. Then we can have a UI test for the failing case.

Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

I think we can improve error message, but otherwise it is good to me

@KiChjang KiChjang added B3-apinoteworthy and removed B0-silent Changes should not be mentioned in any release notes labels Jun 30, 2021
frame/support/test/pallet/Cargo.toml Outdated Show resolved Hide resolved
frame/support/test/pallet/Cargo.toml Outdated Show resolved Hide resolved
@gui1117
Copy link
Contributor

gui1117 commented Jul 16, 2021

bot merge

@ghost
Copy link

ghost commented Jul 16, 2021

Trying merge.

@ghost ghost merged commit 254bc43 into master Jul 16, 2021
@ghost ghost deleted the kckyeung/error-on-genesis-no-std branch July 16, 2021 14:07
This pull request was closed.
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants