-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
Support, openapi spec added #458
Conversation
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.
Good work!
No, I removed |
Thank you! Can you add tests for new formats? Also CI is broken. |
@kibertoad everything now good except CI, can you check CI because I have no experience with that or you can guide me for that |
@Adityapanther See https://github.com/fastify/fastify-swagger/pull/458/checks?check_run_id=3455446735 |
ok, thanks |
}) | ||
ajv.addFormat('byte', { | ||
type: 'string', | ||
validate: byteValidation |
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.
At every validation execution, we are creating a new ajv instance and compilation.
We must avoid it.
If you precompile the functions, it should work as well
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.
can you provide example
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
@mcollina no need to use fastify.inject in test |
What this PR is missing to ship are a few tests, one for each of the new formats, that:
Thanks |
Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
Any progress? |
I am currently busy with my company, I will update this sunday |
any update? |
He said he would update it this sunday. Lets be patient |
okay im sorry, i'm new here, but i think its already a month isn't it? @CarterLi |
I was joking... |
@ec2ainun I am quite busy in my life I am trying to manage my work and personal life, if anyone free here he can provide update to this pull otherwise I will provide update after my project finish |
@Adityapanther sorry, i have little knowledge about openapi spec implementation, i just look around how to create good documentation in order to upload file using fastify, and i found this pull request in discussion |
@mcollina please 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.
It does not seem to include all the tests I asked before.
@mcollina added |
I think these tests are still missing. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
issue fixed
#343
#47
Checklist
npm run test
andnpm run benchmark
and the Code of conduct