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

Document Cargo.toml formatting #528

Closed
NickLarsenNZ opened this issue Jan 17, 2024 · 6 comments · Fixed by #535
Closed

Document Cargo.toml formatting #528

NickLarsenNZ opened this issue Jan 17, 2024 · 6 comments · Fixed by #535
Assignees

Comments

@NickLarsenNZ
Copy link
Member

NickLarsenNZ commented Jan 17, 2024

It is good to have consistent formatting for consistency between developers (eg: using rustfmt for ust code) to reduce noise around style in PR reviews.

Automatic formatting is a preference, however exising TOML formatters (as far as we know) apply rules across the whole toml file (and not only Cargo.toml), rather than different rules per section or use case of toml.

For example,

  • we would like [dependencies] to be ordered alphabetically
  • we would like important [project] attributes to be at the top of the section

So, for now we should document how we expect Cargo.toml to look, to avoid it repeatedly coming up in PRs.

@NickLarsenNZ NickLarsenNZ self-assigned this Jan 17, 2024
@NickLarsenNZ
Copy link
Member Author

NickLarsenNZ commented Jan 17, 2024

I found some kind of style guide (for reference): https://github.com/rust-lang/rust/blob/master/src/doc/style-guide/src/cargo.md

Eg:

Version-sort key names within each section, with the exception of the
[package] section. Put the [package] section at the top of the file; put
the name and version keys in that order at the top of that section,
followed by the remaining keys other than description in order, followed by
the description at the end of that section.

So

[package]
name = "crate-name"
version = "0.0.1"
# ... otherwise alphabetically sorted here
description = "this crate does nothing"

@NickLarsenNZ
Copy link
Member Author

It looks like many organizations/projects have the same conundrum. There also seems to be some work underway to bring the formatting into rustfmt: rust-lang/rustfmt#5240

Maybe we just need to wait it out?

@sbernauer
Copy link
Member

I think we can throw a link to https://github.com/rust-lang/rust/blob/master/src/doc/style-guide/src/cargo.md in our guidelines and call it a day until we got rustfmt support, WDYT?

@sbernauer
Copy link
Member

FYI, just saw apache/iceberg-rust#167

@fhennig
Copy link
Contributor

fhennig commented Jan 29, 2024

I'm working on this now: #535

@sbernauer taplo looks interesting but I think we can just wait until it'll be native in rustfmt, not worth the effort to add another tool to our build process. If you think it's worth investigating maybe track it in a new issue.

@sbernauer
Copy link
Member

but I think we can just wait until it'll be native in rustfmt, not worth the effort to add another tool to our build process

100% agreed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants