Skip to content

Try foundry fmt#136

Draft
NIC619 wants to merge 2 commits intomasterfrom
try_foundry_fmt
Draft

Try foundry fmt#136
NIC619 wants to merge 2 commits intomasterfrom
try_foundry_fmt

Conversation

@NIC619
Copy link
Copy Markdown
Contributor

@NIC619 NIC619 commented Jan 10, 2023

No description provided.

@NIC619 NIC619 marked this pull request as draft January 10, 2023 07:38
bytes32 allowFillHash,
address recipient,
FillReceipt fillReceipt
bytes32 indexed orderHash, address indexed maker, address indexed taker, bytes32 allowFillHash, address recipient, FillReceipt fillReceipt
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This creates different result from prettier and fmt: prettier has a problem but fmt does not, though this line does not exceed 160 characters...so I'm not sure why prettier is complaining.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And this difference from fmt results in it updating many function and event declarations. This one is one of the many examples.

@NIC619
Copy link
Copy Markdown
Contributor Author

NIC619 commented Jan 12, 2023

prettier will convert into multiline if number of function param exceed three, even if line length hasn't reach 160 limit.

@pilagod
Copy link
Copy Markdown
Contributor

pilagod commented Jan 13, 2023

Weird unwritten rules 😅

Overall I think prettier is a better fit to our preference for now. We can delay the foundry fmt integration and keep tracking its updates until it is better enough 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants