-
Notifications
You must be signed in to change notification settings - Fork 82
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
Refactor to contracts engine #141
Refactor to contracts engine #141
Conversation
@egonSchiele @sfcgeorge This one is not finished yet, but can you take a look and tell if you like the direction where this PR is going? |
end | ||
|
||
def set_eigenclass_owner | ||
Engine.fetch_from(eigenclass).owner_class = target |
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.
just eigenclass_engine
here
❤️ |
Works for me. Also happy that Contracts::Module can be removed! Since you are working on this, could you please add comments on all the methods? That will make it easy to modify this code next time...it is pretty complex. |
…s is no longer needed (and at all possible)
46975fc
to
ea19e3b
Compare
Finally green on CI. @egonSchiele can you take a look? |
Looks great. Thank you for adding all the comments! |
Refactor to contracts engine
Weirdly, the change did make contracts a little slower. Old:
New:
Not a meaningful slowdown. |
Allows to be as non-intrusive as possible, moving all the functionality effectively to only one method
.__contracts_engine
(which has rich object(s) inside).Still needs some cleanup:
MethodHandler#handle
.Eigenclass
.include Contracts::Module
since now they get all the needed stuff properly.Engine
. Or at least mark some of them as_pseudo_private
. There is a probability, that some of them could be moved to protected (interaction betweenEngine
andEigenclassEngine
).closes #138