Skip to content

style: update formatting rules #1184

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

joshka
Copy link
Contributor

@joshka joshka commented Jun 20, 2025

chore: update formatting rules

Use spaces instead of tabs for indentation, wrap at 100 characters, and
remove the setting that aligns struct fields vertically as this
generally harms readability.

These changes make the code more consistent with the Rust community's
conventions and improve readability by not truncating lines too early.


Justification:

https://www.youtube.com/watch\?v\=V7PLxL8jIl8

This brings things more in line with pretty much every rust project out there. Hard tabs are used in around 0.1% of rust projects:

@joshka joshka requested a review from orhun as a code owner June 20, 2025 23:35
@codecov-commenter
Copy link

codecov-commenter commented Jun 20, 2025

Codecov Report

Attention: Patch coverage is 40.47491% with 1103 lines in your changes missing coverage. Please review.

Project coverage is 41.34%. Comparing base (4f7379a) to head (f30bfb9).

Files with missing lines Patch % Lines
git-cliff/src/lib.rs 0.00% 425 Missing ⚠️
git-cliff-core/src/changelog.rs 47.73% 172 Missing ⚠️
git-cliff-core/src/remote/mod.rs 26.87% 98 Missing ⚠️
git-cliff-core/src/repo.rs 68.33% 70 Missing ⚠️
git-cliff/src/logger.rs 0.00% 62 Missing ⚠️
git-cliff-core/src/config.rs 41.98% 47 Missing ⚠️
git-cliff-core/src/remote/bitbucket.rs 29.79% 33 Missing ⚠️
git-cliff-core/src/remote/gitlab.rs 38.00% 31 Missing ⚠️
git-cliff-core/src/remote/gitea.rs 34.15% 27 Missing ⚠️
git-cliff-core/src/remote/github.rs 35.72% 27 Missing ⚠️
... and 11 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1184      +/-   ##
==========================================
- Coverage   43.00%   41.34%   -1.66%     
==========================================
  Files          22       22              
  Lines        2056     1921     -135     
==========================================
- Hits          884      794      -90     
+ Misses       1172     1127      -45     
Flag Coverage Δ
unit-tests 41.34% <40.48%> (-1.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines -4 to -5
hard_tabs = true
tab_spaces = 4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main change. Everything else is automatic formatting.

@orhun
Copy link
Owner

orhun commented Jun 22, 2025

I was thinking of creating a patch release for these formatting changes after I'm done with the current reviews. Otherwise it might be a bit annoying to rebase this on other PRs.

I always forget to do this though...

@joshka
Copy link
Contributor Author

joshka commented Jun 22, 2025

I was thinking of creating a patch release for these formatting changes after I'm done with the current reviews. Otherwise it might be a bit annoying to rebase this on other PRs.

I always forget to do this though...

Fixing PRs should fairly simple:

git checkout main -- rustfmt.toml
cargo +nightly fmt
git commit -am 'formatting
git push

If you're squashing merges then this shouldn't really be too much of a problem

Copy link
Contributor Author

@joshka joshka left a comment

Choose a reason for hiding this comment

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

To actually apply this, I'd suggest grabbing rustfmt.toml, discarding everything else and rerunning cargo +nightly fmt

git reset --hard main
git reset fdf857422158338c9ec461270b1aabd31a375049 rustfmt.toml
cargo +nightly fmt
git commit -aC fdf857422158338c9ec461270b1aabd31a375049
git push
git reset 541aa64f4e60738b0ff24895b358c3f05a4b841e .git-blame-ignore-revs
# edit the file with the correct hash

@orhun
Copy link
Owner

orhun commented Jun 22, 2025

yes pls do itt

@joshka joshka force-pushed the jm/format-spaces branch from 541aa64 to 87616d0 Compare June 28, 2025 23:10
Use spaces instead of tabs for indentation, wrap at 100 characters, and
remove the setting that aligns struct fields vertically as this
generally harms readability.

These changes make the code more consistent with the Rust community's
conventions and improve readability by not truncating lines too early.
@joshka joshka force-pushed the jm/format-spaces branch from 87616d0 to 51427a4 Compare June 28, 2025 23:13
Copy link
Contributor Author

@joshka joshka left a comment

Choose a reason for hiding this comment

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

I removed the rev ignore file - do this in a followup PR

channel = "nightly"
channel = "stable"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this was only required to make rustfmt use nightly. Nothing in git-cliff seems to require the nightly compiler to work afaik (I may be wrong there)

@joshka joshka changed the title chore: use spaces instead tabs chore: update formatting rules Jun 28, 2025
@joshka joshka changed the title chore: update formatting rules style: update formatting rules Jun 28, 2025
@joshka joshka force-pushed the jm/format-spaces branch from b1a6305 to f30bfb9 Compare June 28, 2025 23:45
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.

3 participants