twilight-rs / twilight

Powerful, flexible, and scalable ecosystem of Rust libraries for the Discord API.
https://discord.gg/twilight-rs
ISC License
687 stars 131 forks source link

Use Rustfmt Nightly #2365

Open laralove143 opened 4 months ago

laralove143 commented 4 months ago

Rustfmt nightly has been pretty stable for me and it introduces rules that we're trying to apply manually, such as imports_granularity, so I think we could use it on the nightly channel to reduce inconsistencies and make PRs easier to make.

Here's a sample rustfmt.toml we could use that aligns with Rust's conventions while enabling nightly features.

Erk- commented 4 months ago

I have been rather annoyed that I had to use it every time I develop on Songbird. Since my editor/rust-analyzer wants to do it one way and the CI wants it in a different. So personally I would not be in favour of this.

vilgotf commented 4 months ago

I too would not like to require nightly rustfmt during development, but it would be nice to run CI with such a ruleset (instead of us manually spotting formatting issues). #1857 introduced something similar for doc examples

BlackHoleFox commented 4 months ago

for another comment similar to Erk's take: Trading off the style consistency of vast swaths of the Rust ecosystem for new/interested contributors in exchange for some specific, minor rules seems like it goes in the wrong direction, imo.

laralove143 commented 3 months ago

I too would not like to require nightly rustfmt during development, but it would be nice to run CI with such a ruleset (instead of us manually spotting formatting issues). #1857 introduced something similar for doc examples

we could also do this, in that case though the flow would go like user pushes changes -> formatting ci fails -> user pushes formatting chanes -> formatting ci maybe succeeds, for prs involving many commits, wouldnt this introduce many formatting commits? this is fine since we squash and merge but i wonder if it'd be the best DX for contributors

if we go that route, i'd suggest recommending contributors to use rustfmt nightly but they'd still have the option to not use it and fix formatting issues post-push

AEnterprise commented 3 months ago

having people push to then have to manually fix them because we use a different linter also sounds like a very frustrating contributor experience. for the docs it kinda makes sense as having those be invalid will cause confusion to users. but formatting is just style and very personal with varying opinions. Yes consistency is nice but it shouldn't be at the cost of significant development complications

and if it's just a recommendation but we're not going to enforce it then it might as well not be there as it'll just create a weird situation where some code follows it stricly, and then some doesn't

laralove143 commented 3 months ago

i agree reporting formatting issues in ci would lead to a bad contribution experience

as for your second point, i'm not saying that we don't try to follow certain formatting, i'm saying that we should and pr reviews should aim to check whether this style is followed, but for contributors to follow our formatting style we can recommend them to use rustfmt nightly and if they don't want to, they can format their code manually, we can also recommend a way for developers to check the format without applying fixes so that they don't have to push and wait to see if the ci passes

with the style provided to the contributors, it wouldn't affect the contributors' experience if we added a check for formatting in our ci, as it would just automate reviewers having to check the format of the code in prs

while i agree that dx comes before consistency, they don't have to be exclusive to each other, we can find ways to improve consistency without worsening dx