Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify imports at the top of a file #2081

Open
ggwpez opened this issue Oct 30, 2023 · 2 comments
Open

Unify imports at the top of a file #2081

ggwpez opened this issue Oct 30, 2023 · 2 comments
Labels
C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I4-refactor Code needs refactoring.

Comments

@ggwpez
Copy link
Member

ggwpez commented Oct 30, 2023

I often see this in some runtime files, where the imports are spread over multiple blocks. The criteria to how these blocks were formed or the logic behind it is not documented and makes it impossible for cargo fmt to remove duplicated or for the developer to find things:

create_runtime_str, generic, impl_opaque_keys,
traits::{AccountIdConversion, AccountIdLookup, BlakeTwo256, Block as BlockT, Verify},
transaction_validity::{TransactionSource, TransactionValidity},
ApplyExtrinsicResult, Permill,
};
use sp_std::prelude::*;
#[cfg(feature = "std")]
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
construct_runtime,

I think it would be smart to not split the imports with a newline, but have them all in one block.
The only exception would be feature gated imports. As structure i propose:

// Normal imports:
use ...;
use ...;
...

// Feature gated imports:
#[cfg(feature = "std")]
use ::{
    // Std imports all in one block
};

Note that this will cause huge merge conflicts with existing MRs, so please check what MRs are affected first.

@ggwpez ggwpez added the I4-refactor Code needs refactoring. label Oct 30, 2023
@altonen
Copy link
Contributor

altonen commented Oct 30, 2023

On the client side, in some files there is a pattern where crate, external and std imports are in their own blocks. I think that's preferable to clumping everything in one block but since there's no guideline, it's not as consistent as it could perhaps be.

@bkchr
Copy link
Member

bkchr commented Oct 30, 2023

Yeah, we should come up with some kind of consistent way for doing this and then putting it into the CONTRIBUTING.md. Personally I think moving all to one block or having blocks split in the way @altonen said is fine to me.

serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 26, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 27, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
bkchr pushed a commit that referenced this issue Apr 10, 2024
@kianenigma kianenigma added the C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I4-refactor Code needs refactoring.
Projects
None yet
Development

No branches or pull requests

4 participants