-
Notifications
You must be signed in to change notification settings - Fork 200
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
feat(question-answer): separate logic to a new module #1040
feat(question-answer): separate logic to a new module #1040
Conversation
Good to see you are taking your part of the modularisation homework :-) Just keep in mind that there were some fixes in question-answer, not yet merged into 0.3.0-pre branch so I think are not yet integrated here |
Good point, will update when 0.3.0-pre is rebased. |
Is there any reason not to merge this before 0.3.0 is released? If it's ready we can merge it right? |
It should be possible. However I have a suggestion and I am not sure if it makes sense. I would like to release this package with the 0.3.0 release and ALSO keep the original module still in core. When we release a 0.3.1 we can take it out. This makes sure that the module is inside core before it is released. If we release core first it will look at the registry and this module does not exist. Does this make sense or is there an easier solution? |
We can't take it out in 0.3.1, as that would be a breaking change and warrant a 0.4.0 release. But I'm also not sure if it makes 100% sense. The packages will be released at the same time, and if you want to use the question answer module you can just add it as as custom module? Why would one be released before the other? |
I agree that 0.3.0 is a good timing to do this. The same with Action Menu module. I can do it directly in #1030 or create a new PR like this one once if it's merged (would make the commit history a bit cleaner I guess). Let me know your thoughts! |
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
c807610
to
464d574
Compare
Signed-off-by: Berend Sliedrecht <61358536+blu3beri@users.noreply.github.com>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
Codecov Report
@@ Coverage Diff @@
## 0.3.0-pre #1040 +/- ##
=============================================
- Coverage 88.24% 88.22% -0.02%
=============================================
Files 680 680
Lines 15873 15852 -21
Branches 2548 2548
=============================================
- Hits 14007 13986 -21
Misses 1861 1861
Partials 5 5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: blu3beri <berend@animo.id>
Signed-off-by: blu3beri <berend@animo.id>
@TimoGlastra @blu3beri, I don't know what you guys think but I'd rather prefer to merge first #1030 and then update this PR with the changes introduced in question-answer module. And in parallel I can extract action-menu so we can get the initial modularisation job done in a clean way before releasing 0.3.0. |
Sounds good to me :). |
Thanks for the merge @genaris ! |
…undation#1040) Signed-off-by: blu3beri <berend@animo.id> BREAKING CHANGE: question-answer module has been removed from the core and moved to a separate package. To integrate it in an Agent instance, it can be injected in constructor like this: ```ts const agent = new Agent({ config: { /* config */ }, dependencies: agentDependencies, modules: { questionAnswer: new QuestionAnswerModule(), /* other custom modules */ } }) ``` Then, module API can be accessed in `agent.modules.questionAnswer`.
Separated the question-answer logic to a new module.
Since it is now a separate module, if core gets released before the question-answer module it will not be included in the core anymore. This is a breaking change and might cause some headaches.
We can just merge it in after 0.3.0 is released and take our time with a strategy to add it back into the core for now.
I could also keep the question-answer module inside the core and release this one separately. When we do a new 0.3.1 or 0.4.0 release the can remove it and make it dependent on this one.