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

feat: use .rustfmt.toml for formatting options #5899

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Mar 5, 2025

We previously did this with the cargo fmt-stacks command because these config options were not supported in .rustfmt.toml, but they are now supported by the nightly build. Using this directly is better than the alias because we do not need to remember to run the alias.

We previously did this with the `cargo fmt-stacks` command because these
config options were not supported in .rustfmt.toml, but they are now
supported by the nightly build. Using this directly is better than the
alias because we do not need to remember to run the alias.
@obycode obycode requested review from a team as code owners March 5, 2025 18:54
@obycode obycode added this to the 3.1.0.0.8 milestone Mar 5, 2025
@obycode
Copy link
Contributor Author

obycode commented Mar 5, 2025

Ah, right. This will also need a change in the rust format CI action...

@obycode obycode requested a review from a team as a code owner March 5, 2025 19:01
@obycode
Copy link
Contributor Author

obycode commented Mar 5, 2025

This may need a change in the actions repo. I will come back to it later.

@obycode obycode requested a review from wileyj March 5, 2025 19:13
@obycode obycode marked this pull request as draft March 5, 2025 19:13
@wileyj
Copy link
Collaborator

wileyj commented Mar 5, 2025

Ah, right. This will also need a change in the rust format CI action...

https://github.com/stacks-network/actions/blob/main/rustfmt/action.yml

we created this composite because of the alias - i wonder if we can either fork the public workflow, or simply use the public one again

@obycode
Copy link
Contributor Author

obycode commented Mar 7, 2025

Any way we can enforce "nightly" here?

@obycode
Copy link
Contributor Author

obycode commented Mar 7, 2025

Any way we can enforce "nightly" here?

I don't see a way to force nightly for fmt only.

@obycode
Copy link
Contributor Author

obycode commented Mar 7, 2025

My latest commit puts fmt-stacks back for now, since there doesn't seem to be a way to force cargo fmt to use the nightly version. Let's revisit removing fmt-stacks once those options are supported in the stable release.

@obycode obycode marked this pull request as ready for review March 7, 2025 18:49
Copy link
Collaborator

@wileyj wileyj left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: In Review
Development

Successfully merging this pull request may close these issues.

3 participants