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

Proposal to extend the default rustfmt.toml for nightly version #14

Open
wants to merge 1 commit into
base: emilyaherbert/formatting
Choose a base branch
from

Conversation

xgreenx
Copy link

@xgreenx xgreenx commented Aug 24, 2022

The nightly rustfmt contains many new fields that make the code more readable and easy to review.

Initially, I proposed that rustfmt.toml in fuel-core. And after the discussion, we realized maybe better to use it for all Rust projects.

It continues @emilyaherbert's idea and proposal to extend her RFC.

CC please anyone who may be interested in the review=)

Copy link

@emilyaherbert emilyaherbert left a comment

Choose a reason for hiding this comment

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

We shouldn't base the formatting guidelines on nightly rust as our guidelines prefer stable. See: #6 (comment)

@xgreenx
Copy link
Author

xgreenx commented Aug 25, 2022

We shouldn't base the formatting guidelines on nightly rust as our guidelines prefer stable. See: #6 (comment)

Usage nightly formatting doesn't mean building the project with nightly=)

It replaces cargo fmt by cargo +nightly fmt in the terminal=) Each IDE supports a separate channel for formatting, so it will not affect developments at all.

Most fields in the nightly rustfmt were implemented long ago and tested well. New fields format almost all code than the stable version. Not formatting all code can cause changes like this because of the preferences of each developer or local fmt settings.

@mitchmindtree
Copy link

It replaces cargo fmt by cargo +nightly fmt in the terminal=) Each IDE supports a separate channel for formatting, so it will not affect developments at all.

Fwiw, I believe the cargo +nightly fmt syntax requires that the user has installed Rust and cargo via rustup as the +nightly argument hooks into rustup's "proxy" behaviour in order to select the toolchain.

While most users install Rust via rustup, there are many users that don't use rustup at all and prefer to get Rust from other places, e.g. their OS or distribution's official package manager (myself included).

I can appreciate the desire for nicer formatting, but in my experience using anything other than the default tends to open the gates for needless bike-shedding as everyone has their own aesthetic preferences. Personally, I don't think tweaking the formatter slightly is worth the workarounds non-rustup users will require.

@emilyaherbert
Copy link

I can appreciate the desire for nicer formatting, but in my experience using anything other than the default tends to open the gates for needless bike-shedding as everyone has their own aesthetic preferences.

I feel about the same IMO.

How about this @xgreenx. I propose that we close this PR, and that you take over pushing #8 through. How does that sound?

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 this pull request may close these issues.

None yet

3 participants