-
Notifications
You must be signed in to change notification settings - Fork 52
chore: chunked snippet experiment with snippets inside docs #653
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
|
✅ Deploy Preview for openpayments-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
26535d1 to
bb33084
Compare
| @@ -0,0 +1 @@ | |||
| export const VERSION = 'v1' | |||
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.
We can have a version in:
- src/docs/content/version.ts (for latest)
- src/docs/content/1.0/version.ts (for v1)
This version can be used with the new ChunkedSnippet component which will help to detect the correct snippet associated with the major version:
<ChunkedSnippetExperimental source="token/token-revoke.ts" chunk="3" l="node" version={VERSION} />At the moment:
l: language (i.e. node, rust, etc...) (NB: needs a better name, it was namedlsincelangandlanguagewere already used in the component)source: the file source relative to the root of the snippets folderversion: by providing the version the component will read the correct version for the snippets
The path looks lile this: <git-root>/snippets/${version}/${language}/${source}
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.
CC @mkurapov
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.
What do you think if we did version without the v, to allow for minor version + keep inline with the docs as well?
eg <git-root>/snippets/1.0/node/${source}
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 am okay with both approaches. But, thinking more about it, would it be better to name the versions like 1.x 2.x if we only care about versioning major releases? This makes it clearer that we're referring to a major version, rather than a specific release.
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.
Bringing more context:
Me and Max played with versioning a bit and we noticed that the OpenAPI definitions are not versioned by default. In the Astro config we will have to manually add the entries for 1.0/openapi/.....
One thing that we found out is that we will not be able to use dots for the path (1.0/openapi/...) and we will have to go for something like v1/openapi/.....
| const gitRoot = execSync('git rev-parse --show-toplevel').toString().trim(); | ||
| const snippetsPath = path.join(gitRoot, 'snippets'); | ||
|
|
||
| const getChunk = async () => { | ||
| const data = await fs.readFile(path.join(snippetsPath, version, l, source), {encoding: "utf8"}) | ||
| return data | ||
| } |
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 the only code change that happened in this file - how we retrieve the source: readFile instead of a HTTP request.
|
@mkurapov @raducristianpopa Is this OK to close? |
|
Closing as completed in #689 |
EXPERIMENT based on the discussion that took place on 30th of July.