-
-
Notifications
You must be signed in to change notification settings - Fork 753
Promise based helper methods #3465
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
Conversation
DavertMik
left a comment
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.
@nlespiaucq awesome! Just one question here:
runok.js
Outdated
| console.log('Generate TypeScript definition'); | ||
| await npx('jsdoc -c typings/jsdoc.conf.js'); | ||
| await npx('jsdoc -c typings/jsdocPromiseBased.conf.js'); | ||
| fs.renameSync('types.d.ts', 'typings/promiseBasedTypes.d.ts'); |
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.
Maybe we should execute here:
await npx('jsdoc -c typings/jsdoc.conf.js');
so types.d.ts to be actually generated for classical setup?
because according to the code, you generate types twice, overwrites them on second run and move them, but the result of 'jsdoc -c typings/jsdoc.conf.js' is overwritten
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, you're right - I will fix it
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.
Well, it's confusing, but it was done on purpose.
In fact, the 2nd command generates the file types.d.ts on another location, on the root of the folder (to avoid to overwritten the first one). It's why I rename and move this file.
I will have some comments and make it more consistent.
0e12914 to
e08ab86
Compare
e08ab86 to
1f9320a
Compare
DavertMik
left a comment
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.
This was huge!
Thanks, @nlespiaucq for making it happen!
Motivation/Description of the PR
fullPromiseBasedto refer the promise-based definition.Applicable helpers:
Applicable plugins:
Type of change
Checklist:
npm run docs)npm run lint)npm test)