Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/styles/Sei/Headings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ exceptions:
- Sei Network
- SeiDB
- EVM
- ERC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] The change looks right and matches the existing convention for EVM/NFT/RPC/etc. One thing worth confirming from the Vale run you did: the description lists evm/tokens.mdx:14 (#### ERC721 NFTs) among the cleared warnings, but that heading uses the unhyphenated ERC721. If Vale matches exceptions as whole tokens (\bERC\b), ERC721 would not match — there is no word boundary between C and 7 — and that warning should not have cleared. It clearing implies exception matching is substring-based, which would also mean ERC exempts any word containing an uppercase ERC (harmless in practice: no realistic heading word other than an ERC standard contains uppercase ERC).

Not blocking either way — this rule is level: warning and prose-style.yml runs with fail_on_error: false, so nothing gates on it. Just worth a sentence in the PR body clarifying which matching semantics you observed, since it determines whether future unhyphenated ERCnnn headings are covered by this entry or need their own.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, and your inference is right: matching is substring-based, not \bERC\b. I tested it by running Vale over a probe file against two copies of the styles directory, identical except for this one line.

Flagged without the entry, clean with it:

  • ## ERC20 tokens
  • ## ERC721 nfts
  • ## ERC721 NFTs

Still flagged with the entry: ## ERC20 Tokens, ## ERC-20 Tokens, ## ERC Tokens, ## EVM Tokens, ## Foo Tokens.

So unhyphenated ERCnnn is covered by this single entry, and future ERCnnn headings will not need their own. I have added a paragraph to the PR description recording those semantics.

Your follow-up point deserves a correction on my side, though. My original description claimed the entry "does not hide genuine title-case violations," and that was too strong. It is not perfectly surgical: Batching ERC-20 Reads with viem and the three ERC-nnn Interaction titles each have a second capitalized word, and they stop being annotated too. Worth noting that imprecision is not specific to this entry, since ## Foo Interaction never warned either, so the rule was already lenient about a trailing capitalized word in short headings.

That leaves a choice I am happy to defer to you. Either keep it as is, accepting that four imperfect pre-existing headings go unannotated, or I fix those four headings' capitalization in this PR so the rule stays strict. Say which you prefer.

- CosmWasm
- Cosmos
- Ethereum
Expand Down
Loading