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.

Sign up for GitHub

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

Jump to bottom

use cargo-tomlfmt to check Cargo.toml formatting in CI #2033

Merged
merged 4 commits into from
Mar 21, 2022
Merged

use cargo-tomlfmt to check Cargo.toml formatting in CI #2033

merged 4 commits into from
Mar 21, 2022

Conversation

WinkerDu
Copy link
Contributor

@WinkerDu WinkerDu commented Mar 18, 2022 •
edited
Loading

Which issue does this PR close?

Closes #.

Rationale for this change

Datafusion has no Cargo.toml formatting check yet. This pr uses cargo-tomlfmt tool in CI to do it

What changes are included in this PR?

  • added the Github action in rust workflow
  • revise existing Cargo.toml formatting according to cargo-tomlfmt

Are there any user-facing changes?

No.

@WinkerDu
Copy link
Contributor Author

cc @xudong963 , can you please approve running workflows?

@xudong963 xudong963 changed the title fix a typo for ballista Cargo.toml beautify the formatting in ballista Cargo.toml Mar 19, 2022
xudong963
xudong963 approved these changes Mar 19, 2022
View reviewed changes
Copy link
Member

@xudong963 xudong963 left a comment

Choose a reason for hiding this comment

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

Thank you @WinkerDu

@liukun4515
Copy link
Contributor

@WinkerDu Thanks for your contributions.
Can you help to check other toml files for the format in this pull request?

@WinkerDu
Copy link
Contributor Author

@liukun4515 OK, I'd like to check later.

@yjshen
Copy link
Member

yjshen commented Mar 19, 2022

It would be best if we could introduce tools like cargo-tomlfmt to fix existing problems and prevent further violations. As done for markdown in #453.

@WinkerDu
Copy link
Contributor Author

@yjshen Thank you. This idea sounds cool, I'd try this tool in CI workflows

@github-actions github-actions bot added the datafusion Changes in the datafusion crate label Mar 19, 2022
@WinkerDu WinkerDu changed the title beautify the formatting in ballista Cargo.toml use cargo-tomlfmt to check Cargo.toml formatting in CI Mar 19, 2022
@WinkerDu
Copy link
Contributor Author

WinkerDu commented Mar 19, 2022 •
edited
Loading

@yjshen @xudong963
Now the Cargo.toml check is automatic and work well in my fork repo CI. Neat.
Could anybody help starting running CI workflows here?

houqp
houqp reviewed Mar 19, 2022
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
houqp
houqp reviewed Mar 19, 2022
View reviewed changes
.github/workflows/rust.yml Outdated Show resolved Hide resolved
@WinkerDu
Copy link
Contributor Author

@liukun4515 PTAL

yjshen
yjshen approved these changes Mar 21, 2022
View reviewed changes
Copy link
Member

@yjshen yjshen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! Thanks @WinkerDu

@yjshen yjshen merged commit d02d969 into apache:master Mar 21, 2022
@WinkerDu WinkerDu deleted the fix-ballista-cargo-typo branch March 21, 2022 03:42
@xudong963
Copy link
Member

Thanks @WinkerDu

@yjshen yjshen mentioned this pull request Apr 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liukun4515 liukun4515 liukun4515 left review comments

@xudong963 xudong963 xudong963 approved these changes

@yjshen yjshen yjshen approved these changes

@houqp houqp houqp approved these changes

Assignees
No one assigned
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@WinkerDu @liukun4515 @yjshen @xudong963 @houqp