Update generate-docs to modern JS #874
Conversation
docgen/generate-docs.js
Outdated
| const files = await fs.readdir(docPath); | ||
| console.log(files); | ||
| const renames = []; | ||
| files.forEach(file => { |
There was a problem hiding this comment.
Nit since it was already like this: https://google.github.io/styleguide/tsguide.html#iterating-containers suggests that we should avoid .foreach and use for loops instead.
joehan
left a comment
joehan
left a comment
There was a problem hiding this comment.
LGTM with a few suggestion to modernize further if we want.
docgen/generate-docs.js
Outdated
| console.error(err); | ||
| } | ||
| } | ||
| })(); |
There was a problem hiding this comment.
Add newline at end of file.
docgen/generate-docs.js
Outdated
| const renames = []; | ||
| files.forEach(file => { | ||
| let newFileName = file; | ||
| if (_.startsWith(file, "_") && _.endsWith(file, "html")) { |
There was a problem hiding this comment.
Nit - this is the only usage of lodash, and it can be done in native JS pretty simply.
inlined
force-pushed
the
inlined.update-docgen
branch
from
eaeb2c1 to
b03a948
Compare
|
NVM I found the bug. The old code used a global variable and I declared a local with the same name. I've removed the global. |
|
Glad to see you folks keeping this fresh and up to date! Assuming this doesn't change anything about how to run the script to generate output. If it does, or if it requires a node update, please LMK. Thanks! |
|
@egilmorez The script returns warnings when it's run, though I've made sure there aren't any regressions. Do you want to verify that things are working correctly? |
|
You bet, I'll put that on my list. |
4 participants
While reviewing
docgen/generate-docs.jsI had a little trouble getting my mind to think in such old JS 🙃Updated to use async/await. In the meantime I caught a case where we weren't awaiting a promise.