-
Notifications
You must be signed in to change notification settings - Fork 917
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
Refactor: types/interfaces #627
Conversation
Also some stuff from #232 (comment):
maybe are you talking about PR #515 ? |
Oh yes!! Done in 07d1a04 I also tweaked some comments... and thought a bit about the JSDocs comments. Since we are using TS, I feel that some JSDocs annotations are redundant. I personally will only use them if:
I removed some of them in the PR, but applying these rules we could probably remove a bunch more haha. What do you all think?
Yesss thanks!! |
💯 agree with this!!! Lol I was planning to mention it in my previous comment to remove stuff like this :
|
Yep, maybe another PR! Added it as a ToDo in #617 |
packages/nextjs/components/scaffold-eth/Contract/utilsContract.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging this, Thanks Carlos 🙌
Tries to solve a couple of items from (+ general cleanup) #617
T
prefix on typesexport
from types when they weren't usedWe can add more stuff in this PR (if it's in the same line).
Also, do you remember where was the discussion about viem's Address vs
0x${string}
? We are still overriding inabi.d.ts
, and would like to give it a second thought.Thanks!