-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: add sorbet type checker #83
Conversation
peterdeme
commented
Feb 9, 2022
- Add types. Runtime check is disabled, but static check is enabled.
- Add type checking to CI
3cedae5
to
e0de8f8
Compare
# We will enable it with a major bump in the future, | ||
# but for now, let's just run a static type check. | ||
|
||
StringKeyHash = T.type_alias { T::Hash[T.any(String, Symbol), T.untyped] } |
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.
We had issues with having strings/symbols in maps before, would you think we should allow only symbols here in next major version?
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.
Agreed. Want me to modify this to this?
SymbolKeyHash = T.type_alias { T::Hash[Symbol, T.untyped] }
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.
Yes, I think it would be right move.
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.
@ffenix113 I looked into it. Unfortunately we rely on string in many places already so right now I wouldn't make that change. Right before bumping to the next major, sure.
657e260
to
08ee246
Compare
08ee246
to
98106c3
Compare