Skip to content

use auto-formatting #305

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

Merged
merged 2 commits into from
Nov 11, 2022
Merged

use auto-formatting #305

merged 2 commits into from
Nov 11, 2022

Conversation

danieleades
Copy link
Contributor

the benefits of auto-formatting are many and varied, and are well-documented

summarily,

  • it creates a consistent visual style across the ecosystem (rustfmt is almost universally adopted)
  • reduces diff noise (primarily by allowing a contributor to autoformat their changes without affecting other code)
  • obviates the need to ever discuss formatting in a PR again

I would personally strongly advocate for using the default config, but if there are any consistent differences in style from the default then these can be configured.

@dwrensha
Copy link
Member

dwrensha commented Nov 11, 2022

Is there a way to exclude the generate files capnpc/src/schema_capnp.rs, capnp-rpc/src/rpc_capnp.rs and capnp-rpc/src/rpc_twoparty_capnp.rs from this formatting?

@danieleades
Copy link
Contributor Author

danieleades commented Nov 11, 2022

Is there a way to exclude the generate files capnpc/src/schema_capnp.rs, capnp-rpc/src/rpc_capnp.rs and capnp-rpc/src/rpc_twoparty_capnp.rs from this formatting?

i'll look into it. Why these files specifically?

anything with @generated should be skipped by default, i think

EDIT: not 100% sure what's happening here. according to this PR, @generated files should be excluded - rust-lang/rustfmt#4877

EDIT: got it. despite what the PR said, the config option was disabled by default. have added a rustfmt config file

@danieleades danieleades force-pushed the rustfmt branch 2 times, most recently from 0de586a to b81a4c8 Compare November 11, 2022 16:19
@dwrensha
Copy link
Member

Nice, and thanks @Timmmm for adding @generated in #153!

@dwrensha dwrensha merged commit 77a3b75 into capnproto:master Nov 11, 2022
@danieleades danieleades deleted the rustfmt branch November 11, 2022 19:37
ErikMcClure pushed a commit to Fundament-Software/capstone-rs that referenced this pull request Mar 25, 2024
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