-
Notifications
You must be signed in to change notification settings - Fork 383
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: implement GET /documents (closes #18) #24
Conversation
@@ -4,7 +4,7 @@ | |||
"azureFunctions.projectLanguage": "TypeScript", | |||
"azureFunctions.projectRuntime": "~4", | |||
"debug.internalConsoleOptions": "neverOpen", | |||
"azureFunctions.projectLanguageModel": 4, | |||
"azureFunctions.projectLanguageModel": 3, |
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.
Are we really going to change it to 3? Because I remember we used to change versions for debugging purposes, right?
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.
Yes, until the issue with the extension is fixed it's best to use the working version
return Buffer.concat(chunks); | ||
} | ||
|
||
app.get('documents', { |
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.
if you wish, here I can separate and place this route in the index.ts
in the root of the api
folder when I do the Protocol
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.
I just readed the comment above now. Ok. Let's keep it this way. In the next PR for Protocol
I will change the chat
api and then the .md
files in the tutorial
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.
LGTM
Purpose
@glaucia86 I extracted the api declaration from
packages/api/src/index.ts
to keep everything in the function to keep the code simpler as we discussed (chat can be done later).Just to reiterate: what you did initially is the best practice that should be used for larger projects, this is only done to keep the projet simpler 🙂 .
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?