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

Change all type aliases to type definitions #207

Open
samlaf opened this issue Apr 19, 2024 · 4 comments
Open

Change all type aliases to type definitions #207

samlaf opened this issue Apr 19, 2024 · 4 comments
Labels
good first issue Good for newcomers p3 low priority

Comments

@samlaf
Copy link
Collaborator

samlaf commented Apr 19, 2024

We shouldn't have used type aliases in types (see for example OperatorId). See quote below.

Problem it creates is that when another pkg uses the sdk and calls a function which returns an Aliased type, vscode (and probably other editors?) show the returned type as the underneath type definition (instead of the type alias name).

So for eg getOperatorsAvsStateAtBlock will show
image
instead of its actual definition in the sdk as
image

Note the key type is Bytes32 instead of OperatorId, so it lost its semantics.

Type aliases are not meant for everyday use. They were introduced to support gradual code repair while moving a type between packages during large-scale refactoring. Codebase Refactoring (with help from Go) covers this in detail.

From https://yourbasic.org/golang/type-alias/ (and see the linked https://go.dev/talks/2016/refactor.article)

@samlaf samlaf added good first issue Good for newcomers p3 low priority labels Apr 19, 2024
@0xpanicError
Copy link

0xpanicError commented Apr 22, 2024

Hey @samlaf can I take up this issue?

@samlaf
Copy link
Collaborator Author

samlaf commented Apr 22, 2024

Hey @samlaf can I take up this issue?

yes sir

@mohitsethia
Copy link

Hey @samlaf , I see that it is still not resolved. Can I contribute to it? And do I need to change all the type alias to the actual datatypes?

@samlaf
Copy link
Collaborator Author

samlaf commented Jul 10, 2024

Hey @samlaf , I see that it is still not resolved. Can I contribute to it? And do I need to change all the type alias to the actual datatypes?

There’s a PR open for it, I haven’t had time to review it though :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers p3 low priority
Projects
None yet
Development

No branches or pull requests

3 participants