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

chore: use taplo to format Cargo.toml manifests #9263

Merged
merged 7 commits into from
Feb 20, 2024

Conversation

waynexia
Copy link
Member

Which issue does this PR close?

Closes #9262.

Rationale for this change

See #9262 for details.

What changes are included in this PR?

Changes:

  • Add taplo to CI
  • Format existing Cargo.toml files
  • Add a configuration file
  • Update related docs

To install taplo, run cargo install taplo-cli. Or by other ways in document.

To format toml files, use taplo format. It would read config file taplo.toml from this PR.

Are these changes tested?

no need

Are there any user-facing changes?

For developers, yes.

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@github-actions github-actions bot added documentation Improvements or additions to documentation sql SQL Planner development-process Related to development process of DataFusion logical-expr Logical plan and expressions physical-expr Physical Expressions optimizer Optimizer rules core Core DataFusion crate substrait labels Feb 18, 2024
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

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

Won't lie, it bugs me a little that name is no longer the first key in this new format 😅

I guess it is better to have some defined order for keys, but can this be configured?

Unless others are fine with the default alphabetical 👍

This reverts commit 1819daa.
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@github-actions github-actions bot removed documentation Improvements or additions to documentation sql SQL Planner logical-expr Logical plan and expressions substrait labels Feb 19, 2024
@waynexia
Copy link
Member Author

Thanks for reviewing this @Jefffrey

I added a rule to skip reordering package table, it should look better now. Please check it out 😆

@waynexia waynexia changed the title Taplo format chore: use taplo to format Cargo.toml manifests Feb 19, 2024
Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

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

Nice 🚀

@waynexia waynexia merged commit cf92f3b into apache:main Feb 20, 2024
25 checks passed
@waynexia waynexia deleted the taplo-format branch February 20, 2024 02:36
@Omega359
Copy link
Contributor

This broke /dev/rust-lint.sh because

/ci/scripts/rust_toml_fmt.sh isn't valid bash:

set -ex
taplo format
done <-- this should be removed.

@waynexia
Copy link
Member Author

Ahhh my bad 😵‍💫 Are you willing to submit a fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate development-process Related to development process of DataFusion optimizer Optimizer rules physical-expr Physical Expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: replace toml formatter with taplo
3 participants