-
Notifications
You must be signed in to change notification settings - Fork 27
Remove use of module_function #150
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
6a01c0c to
4fc00a6
Compare
| # or nil if it can't find anything | ||
| sig { params(klass: T.nilable(T.any(T::Class[T.anything], Module))).returns(T.nilable(String)) } | ||
| def path_from_klass(klass) | ||
| sig { params(klass: T.nilable(T.any(T::Class[T.anything], T::Module[T.anything]))).returns(T.nilable(String)) } |
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.
Module is generic in recent sorbet releases, and required at typed: strict: sorbet/sorbet#9580
| @@ -1,4 +1,4 @@ | |||
| # typed: false | |||
| # typed: strict | |||
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.
This is an easy type promotion 📈
| let(:codes_team) { instance_double(CodeTeams::Team) } | ||
|
|
||
| before do | ||
| allow(codes_team).to receive(:is_a?).with(CodeTeams::Team).and_return(true) |
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.
This is needed bc runtime typechecking is enabled as a result of removing module_function. (See sorbet/sorbet#8531 linked in the PR summary.)
| spec.add_dependency 'code_teams', '~> 1.0' | ||
| spec.add_dependency 'packs-specification' | ||
| spec.add_dependency 'sorbet-runtime', '>= 0.5.11249' | ||
| spec.add_dependency 'sorbet-runtime', '>= 0.6.12763' |
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.
A more recent release is necessary for T::Module to resolve (see comment below).
4fc00a6 to
67bb6ea
Compare
67bb6ea to
07c898b
Compare
sorbet-runtime does not work on
sigs defined on module_function methods, when called as class methods: sorbet/sorbet#8531tapioca also fails to include type information for the class method versions when generating gem shims: Shopify/tapioca#2000
AFAICT, the instance version of methods created with
module_functionare not used, so this PR proposes using the class definitions only. (This ofc may not be true for dependents, so this is very much a breaking change.)There are some other light changes for compatibility, which i'll comment on below.