-
Notifications
You must be signed in to change notification settings - Fork 78
feat(ruby): add static type checking with Sorbet #585
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
Conversation
AFOliveira
left a comment
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.
The changes look good to me, aside from that small CSR which seems misplaced in the PR.
I do feel like the bar for contributing to UDB’s codebase is a bit higher than it needs to be. It seems difficult for folks who haven’t worked on the project before to make even small changes. I think that’s part of why there’s a lot of interest in UDB, but not as many people actively contributing. As long as we're around and maintaining it, things should be fine—but I do wonder if this might cause issues in the long run.
I don’t have strong opinions on this. I’m not familiar with Sorbet yet, but I’ll learn it. The fact that it’s not mandatory right now feels like a good way to ease into it.
ThinkOpenly
left a comment
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.
I'm more of a rube than a Ruby programmer, but doesn't seem too bad, especially if adoption for new code is currently optional.
The recommended way to handle rbi files for gems is to check them in. However, since we use a container enviornment and can link to a verion of hand-written rbis, we don't need to do this.
|
@dhower-qc should mip get his own issue? |
Ruby, like most scripting languages, is not statically typed. While great for initial productivity, it can make changes challenging since errors are only caught at runtime, and only if that path is exercised.
There are two type checking solutions for Ruby: Sorbet and RBS/Steep. I tried both with the UDB code base. Based on that evaluation, I determined that Sorbet was higher quality, had much better documentation, and more mature. It's also in commercial use so is likely to be supported long-term.
Sorbet uses gradual type checking: you can add type signatures to subsets of the code, and Sorbet only checks what is known. Thus, we can start using it without annotating the entire code base up front.
This PR highlights the utility of the tool: it found several bugs already.
There is a cost to using Sorbet, mostly in terms of added development time
Let's discuss more here, but based on this experiment my recommendation is that we merge this PR.