Skip to content
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

trying to get generate-doc-examples.js working #2284

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

anniegale9538
Copy link

Hi Hello!

I've been contributing to the Python doc examples in the Elasticsearch-py repo (with Quentin Pradet's help) and am trying to get the doc examples for the other languages available!

Disclaimer: I'm not a JS or TypeScript developer, but wanted to take a whack at getting a code change up.

The alternative_reports.json file that used to live in the client_teams repo looks to have moved to the client-fliight-recorder repo and is now using the parsed_alternative_report.json file to generate the doc examples.

I was able to get this working making a few changes to the generate_doc_examples.js file.

I was unable to get it working used node, but was able to get it running using 'npx tsx' (You may hate this and I'm sorry!)

I was able to get the doc examples generating locally, but didn't want to add them and clutter this initial PR.

Copy link
Member

@JoshMock JoshMock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for making the change. 👏

package.json Outdated Show resolved Hide resolved
Comment on lines +80 to +81
"ts-standard": "11.0.0",
"typescript": "4.6.4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"ts-standard": "11.0.0",
"typescript": "4.6.4",
"ts-standard": "^11.0.0",
"typescript": "^4.6.4",

I'd switch back to what was already there just to keep things consistent. Pinning to an exact version is not ideal unless absolutely necessary.

Comment on lines +143 to +151
const { results } = await new Promise((resolve, reject) => {
tsStandard.lintText(code, { fix: true }, (err, result) => {
if (err) {
reject(err)
} else {
resolve(result)
}
})
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to wrap this explicitly in a Promise. Since you made generateAsciidoc an async function, any errors thrown by lintText will be thrown just like any other exception would be.

Suggested change
const { results } = await new Promise((resolve, reject) => {
tsStandard.lintText(code, { fix: true }, (err, result) => {
if (err) {
reject(err)
} else {
resolve(result)
}
})
})
const { results } = await tsStandard.lintText(code, { fix: true })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants