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

Code formatting with rustfmt.toml config #5684

Open
kamuik16 opened this issue Nov 21, 2024 · 4 comments · May be fixed by #5686
Open

Code formatting with rustfmt.toml config #5684

kamuik16 opened this issue Nov 21, 2024 · 4 comments · May be fixed by #5686

Comments

@kamuik16
Copy link
Contributor

Description

We can add a rustfmt.toml file to the repo and use some of the nightly features like

reorder_imports  = true
imports_granularity = "Crate"
format_code_in_doc_comments = true

Read more about these configs in here.

Motivation

Highly improved code formatting and readability.
Formatting imports.
Formatting the code in doc as well.
Example: reth - rustfmt.toml

Current Implementation

This:

pub use self::behaviour::{Behaviour, Event, MessageAuthenticity};
pub use self::config::{Config, ConfigBuilder, ValidationMode, Version};
pub use self::error::{ConfigBuilderError, PublishError, SubscriptionError, ValidationError};
pub use self::metrics::Config as MetricsConfig;
pub use self::peer_score::{
    score_parameter_decay, score_parameter_decay_with_base, PeerScoreParams, PeerScoreThresholds,
    TopicScoreParams,
};
pub use self::subscription_filter::{
    AllowAllSubscriptionFilter, CallbackSubscriptionFilter, CombinedSubscriptionFilters,
    MaxCountSubscriptionFilter, RegexSubscriptionFilter, TopicSubscriptionFilter,
    WhitelistSubscriptionFilter,
};
pub use self::topic::{Hasher, Topic, TopicHash};
pub use self::transform::{DataTransform, IdentityTransform};
pub use self::types::{Message, MessageAcceptance, MessageId, RawMessage};

will look like this:

pub use self::{
    behaviour::{Behaviour, Event, MessageAuthenticity},
    config::{Config, ConfigBuilder, ValidationMode, Version},
    error::{ConfigBuilderError, PublishError, SubscriptionError, ValidationError},
    metrics::Config as MetricsConfig,
    peer_score::{
        score_parameter_decay, score_parameter_decay_with_base, PeerScoreParams,
        PeerScoreThresholds, TopicScoreParams,
    },
    subscription_filter::{
        AllowAllSubscriptionFilter, CallbackSubscriptionFilter, CombinedSubscriptionFilters,
        MaxCountSubscriptionFilter, RegexSubscriptionFilter, TopicSubscriptionFilter,
        WhitelistSubscriptionFilter,
    },
    topic::{Hasher, Topic, TopicHash},
    transform::{DataTransform, IdentityTransform},
    types::{Message, MessageAcceptance, MessageId, RawMessage},
};

This is just one of the example across repo.

Are you planning to do it yourself in a pull request ?

Yes

@kamuik16
Copy link
Contributor Author

Curios to know the opinions of the maintainers here :)

@jxs
Copy link
Member

jxs commented Nov 22, 2024

Hi, there was one, introduced in #18, which was then removed in #137.
No strong opinion, just wouldn't like something that changes the code style too much with lots of diffs. If it's mostly imports makes sense to me

@kamuik16
Copy link
Contributor Author

Hi, there was one, introduced in #18, which was then removed in #137. No strong opinion, just wouldn't like something that changes the code style too much with lots of diffs. If it's mostly imports makes sense to me

yes, related to imports and formatting code in the docs which are in the ///. But we'll have to use the nightly toolchain like cargo +nightly fmt so if it makes sense to you, I can open a PR for it. We'll have to change cargo fmt check in the ci to use the nightly feature. wdyt?

@jxs
Copy link
Member

jxs commented Nov 23, 2024

yeah, thanks for it! With a PR we can then discuss and possible fine tune the changes

@kamuik16 kamuik16 linked a pull request Nov 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants