-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat(scripts): move helpers to snippet files for algolia doc #3197
Conversation
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
a4dd3e5
to
0996a50
Compare
}; | ||
|
||
export const waitForTask = { | ||
csharp: `await client.WaitForTaskAsync("<<indexName>>", response.TaskID);`, |
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.
I've used this format for templating for now but it will most likely evolve from @kai687 's PoC
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 can use the same format as for our CTS, we use ${{localhost}}
for example
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 will change for sure later anyway so... I guess we don't really mind for now
scripts/specs/snippets.ts
Outdated
if (!('waitForAppTask' in snippetSamples[language])) { | ||
snippetSamples[language].waitForAppTask = { | ||
default: waitForAppTask[language], | ||
}; | ||
} | ||
|
||
if (!('waitForApiKey' in snippetSamples[language])) { | ||
snippetSamples[language].waitForApiKey = { | ||
default: waitForApiKey[language], | ||
}; | ||
} | ||
|
||
if (!('waitForTask' in snippetSamples[language])) { | ||
snippetSamples[language].waitForTask = { | ||
default: waitForTask[language], | ||
}; | ||
} |
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 is actually the only bits that changed, lmk if you want 2 PRs instead for the file move
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.
look good ! nice split
scripts/specs/snippets.ts
Outdated
import fsp from 'fs/promises'; | ||
|
||
import { GENERATORS, capitalize, createClientName, toAbsolutePath } from '../common.js'; | ||
import type { CodeSamples, Language, SnippetForMethod, SnippetSamples } from '../types.js'; |
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 move some of those types to the new types.ts
file pls ?
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.
indeed!
}; | ||
|
||
export const waitForTask = { | ||
csharp: `await client.WaitForTaskAsync("<<indexName>>", response.TaskID);`, |
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 can use the same format as for our CTS, we use ${{localhost}}
for example
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.
perfect !
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-2458
Changes included:
duplicates the snippets for our netlify doc to the JSON of the initial snippet, with a template pattern, this allows us to ship a single file to the documentation, which doesn't use JS anyway so it's better if we let them do the templating/variable replace directly
also move the specs logic to its own folder, the file was pretty complicate to navigate in