-
Notifications
You must be signed in to change notification settings - Fork 229
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
Fix/extract annotations error handling #363
Fix/extract annotations error handling #363
Conversation
…epreacted and does not support eslint 8
@nejclovrencic thanks for the pull request. Some tests break. Can you check it, please? |
@daniloab tests should pass now, audit test was failing because of vulnerability in |
@daniloab do you have any ETA when we can get this merged? Can we release a new minor version, including this fix? |
I would like to have a test plan for every merge change. Can we have someone to test your changes? https://dev.to/woovi/test-plan-driven-development-56a2 |
@daniloab I have added the test plans in the PR description. I have tested the change, and will ask someone from my team to test this as well. |
Sure, I agree. I mean tiny steps:
|
I can verify that it works as expected after following the test plan in the PR description. I've created a repo for testing - https://github.com/shekharnwagh/swagger-jsdoc-fix-test. |
this is really cool, thanks @shekharnwagh |
@daniloabyes, this is expected. There is a broken symlink (src/app.js links to itself), so the library with the PR throws the error. If you change
|
@daniloab I added that symlink to create a scenario where we can test the new |
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. |
@daniloab Would it be possible to merge and release this. |
can you check the audit flow, please? https://github.com/Surnet/swagger-jsdoc/actions/runs/5763184661/job/15624477586 |
When running
build
, the for loop goes through each file and callsextractAnnotations
and executes parsing logic. Any of this logic could throw, for examplereadFileSync
insideextractAnnotations
could throw. Currently, this fails the whole process.This PR wraps everything inside the for loop in a try catch, and throws based on
failOnError
options.This PR also replaced
eslint-loader
because it is deprecated andnpm install
on master branch currently fails, aseslint-loader
does not support eslint v8.Test plans
1
failOnError: false
2