Skip to content

Convert file-level rustfmt skip to fn level skips #3809

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

joostjager
Copy link
Contributor

@joostjager joostjager commented May 29, 2025

Extend the approach proposed in #3767 to the full repository. The change set is automatically generated using a script. The script omits skip directives when they would not impact formatting (code already rustfmt-compliant).

I've fixed various problems with the script, and it may be that there are a few edge cases left that should have had a skip directive. There could also be a few redundant ones. I think we can live with that.

@ldk-reviews-bot
Copy link

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

They refer to other files and make rustfmt want to search for those.
This is problematic with the skip-insertion script.
@joostjager joostjager force-pushed the rustfmt-skip-all branch 2 times, most recently from a88062e to 16025f2 Compare May 29, 2025 14:26
@joostjager
Copy link
Contributor Author

As discussed in sync meet, I will try to extend the script to ignore rustfmt changes that only affect the function signature.

Those two directives don't seem to work together properly and make the
build fail.
@joostjager
Copy link
Contributor Author

Pushed fix up commit to ignore sig changes. Modest reduction in skips, with a few edge cases.

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.

2 participants