8000 Fix errors in `T.bind` example by ghiculescu · Pull Request #4787 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
Dismiss alert

Fix errors in T.bind example #4787

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 26, 2021
Merged

Fix errors in T.bind example #4787

merged 2 commits into from
Oct 26, 2021

Conversation

ghiculescu
Copy link
Contributor

Fixes some errors in the complex T.bind example added in #4170. cc @vinistock

Before vs After

  • Changed included example to before_create so it's more similar to the previous example
  • Made create_tag an instance method, otherwise both classes error so it's unclear what the example is showing
  • Fixed typo, Taggeable should be Taggable

@ghiculescu ghiculescu requested a review from a team as a code owner October 26, 2021 19:57
@ghiculescu ghiculescu requested review from froydnj and removed request for a team October 26, 2021 19:57
ghiculescu
ghiculescu commented Oct 26, 2021
View reviewed changes
@jez jez enabled auto-merge (squash) October 26, 2021 20:25
@vinistock
Copy link
Collaborator
vinistock commented Oct 26, 2021 •
edited
Loading

The updated example doesn't really reflect what concerns do though. The block that is passed to a concern's included is executed in the context of the class - not the instance.

You mentioned before_create, which indeed uses the instance context, but I don't see the change related to it.

If we wanted a more complete example, involving a T.bind for both class and instance contexts, we could do something like this

module MyConcern
  extend ActiveSupport::Concern

  included do
    T.bind(self, T.class_of(Post))

    before_create do
      T.bind(self, Post)
      #...
    end
  end
end

@jez jez merged commit 33835d7 into sorbet:master Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jez jez jez approved these changes

@froydnj froydnj Awaiting requested review from froydnj froydnj is a code owner automatically assigned from sorbet/core-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ghiculescu @vinistock @jez
0