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

ci: taplo fmt check for TOML files #645

Merged
merged 3 commits into from
Jan 26, 2025

Conversation

sergerad
Copy link

Closes #644

  • Add taplo commands to Makefile
  • Add taplo job to lint.yml
  • Diffs from running taplo fmt locally

@sergerad sergerad changed the title ci: taplo fmt check for Cargo.toml files` ci: taplo fmt check for TOML files` Jan 26, 2025
@bobbinth bobbinth changed the title ci: taplo fmt check for TOML files` ci: taplo fmt check for TOML files Jan 26, 2025
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Thanks @sergerad; I appreciate you picking this up :)

Could you rebase this to target the next branch? We use next as our dev branch (see here).

.github/workflows/lint.yml Outdated Show resolved Hide resolved
@Mirko-von-Leipzig
Copy link
Contributor

@bobbinth in terms of styling, are there any options that catch your eye?

I propose adding a .taplo.toml configuration file with the following non-defaults:

[formatting]
align_entries = true    # This might not be nice; will have to judge after I see it.
column_width = 120
reorder_keys = true
reorder_arrays = true
reorder_inline_tables = true

Mostly just alphabetical ordering to ensure consistency. Might end up looking bad; note that deps could still be grouped together via newlines.

Something else that's nice is that taplo provides an LSP which is nice for dev work.

.github/workflows/lint.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@bobbinth
Copy link
Contributor

@bobbinth in terms of styling, are there any options that catch your eye?

I think the ones that you've listed make sense and I didn't see anything else that I'd add.

@Mirko-von-Leipzig Mirko-von-Leipzig changed the base branch from main to next January 26, 2025 08:40
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Just the one small change; the rest looks great

Cargo.toml Outdated Show resolved Hide resolved
@Mirko-von-Leipzig Mirko-von-Leipzig merged commit 44394e4 into 0xPolygonMiden:next Jan 26, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter for Cargo.toml files
3 participants