stackabletech / documentation

Stackable's central documentation repository built on Antora
https://docs.stackable.tech
Apache License 2.0
12 stars 11 forks source link

Document Cargo.toml formatting #528

Closed NickLarsenNZ closed 8 months ago

NickLarsenNZ commented 8 months ago

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,

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

NickLarsenNZ commented 8 months ago

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 commented 8 months ago

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

Maybe we just need to wait it out?

sbernauer commented 8 months ago

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 commented 8 months ago

FYI, just saw https://github.com/apache/iceberg-rust/pull/167

fhennig commented 8 months ago

I'm working on this now: https://github.com/stackabletech/documentation/pull/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 commented 8 months ago

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