-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
community[minor]: Add stack exchange tool #5162
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -0,0 +1,180 @@ | |||
import * as querystring from 'querystring'; |
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.
Hey team, I've flagged this PR for your review as it introduces a new fetch request in the _makeRequest
method. This is a net-new, non-api client external HTTP request, and I wanted to bring it to your attention for further review.
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.
Their is no official Stack Exchange NPM package that wrap the API so I used the fetch directly
Thanks for the PR! Could you add a docs page? |
I have added some documentation in the corresponding folder and solve the different feedbacks |
@jacoblee93 Do you need any additional information in order to merge this PR ? |
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.
couple nits, besides that it lgtm! thanks for handling this.
Will merge tmro when I cut a release. Thank you!
Hmm, looks like formatting is failing. Could you run |
Thank you for spotting the format issue @bracesproul . I have fixed them and apply your proposal to have some cleaner jsdoc |
Sorry I think my last commit didn't include the fix will push it asap |
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.
Ah woops, you need to add this new module to libs/langchain-community/langchain.config.js
see docs here
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.
Pushed a commit running yarn build
This is required after you add a new entrypoint b/c it adds it to the package.json
and updates the gitignore
to not commit build artifacts
Thanks though I shouldn’t add the file in the PR ! Will try to make my next PR smoother and thank you for your time on it |
This change add the tools StackExchange as in the Python version of LangChain
Twitter: @dev_lagarde