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

add format checker to .toml #13968

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

georgehao
Copy link

@georgehao georgehao commented Jan 24, 2025

This pr is to resolve the #13821 (comment)

The issue detail:

1. most of the reth Cargo.toml indentation use space, but some files use tab

Image Image

Tab: https://github.com/paradigmxyz/reth/blob/main/crates/ethereum/evm/Cargo.toml#L49
Space: https://github.com/paradigmxyz/reth/blob/main/crates/trie/trie/Cargo.toml#L74

if I open the auto format option in Rustrover, some unexpected formats appear.

suggest use the rule: "Use spaces instead of tabs"

2. some Cargo.toml files have redundant space

reth/Cargo.toml

Line 467 in 1296bac

op-alloy-rpc-types-engine = { version = "0.9.6", default-features = false }

image

Test result

This checker have been tested in scroll repo.

Updates

So add a .toml checker here and add lint-cargo-toml to Makefile

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I already know that this will cause issues because people use different formatters for toml

before we can consider this, this definitely needs a makefile command to format all cargo.tomls and a note in the formatting section, otherwise this will cost me a lot of time when I need to touchup prs.

thoughts on this @DaniPopes

@georgehao georgehao changed the title add format checker to Cargo.toml add format checker to .toml Jan 24, 2025
@georgehao
Copy link
Author

georgehao commented Jan 24, 2025

needs a makefile command to format all cargo.tomls and a note in the formatting section

done

Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

sick!

@emhane emhane added the A-meta Changes in the contributor workflow and planning label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Changes in the contributor workflow and planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants