-
-
Notifications
You must be signed in to change notification settings - Fork 195
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
Finish up optional deps #11
Finish up optional deps #11
Conversation
def self.execute(input:) | ||
raise NotImplementedError, "Your tool must implement the `self.execute(input:)` method that returns a string" | ||
new.execute(input: input) | ||
end |
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.
Tools now work by creating an instance of the tool, but I still keep the class-level method to allow direct execution.
def self.execute_search(input:) | ||
new.execute_search(input: input) | ||
end |
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 considered not adding this, but didn't want to require other tools to instantiate this tool to use it.
This is bigger than I thought it would need to be because of switching tools from effectively singletons to instances. Let me know your thoughts. |
This finishes up applying optional dependencies (proof of concept in #7) and improves code style.