-
Notifications
You must be signed in to change notification settings - Fork 3
Add archive attachments extension #90
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 docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
extensions/archive-attachments.js
Outdated
| * @param {string} tempDir - The temporary directory containing files to archive. | ||
| * @returns {Promise<Buffer>} - A promise that resolves to the tar.gz buffer. | ||
| */ | ||
| function createTarInMemory(tempDir) { |
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.
should this have async?
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's not needed but because it returns a promise, marking it as async makes it more explicit 😄
extensions/archive-attachments.js
Outdated
|
|
||
| // Asynchronously create the tar.gz archive in memory | ||
| logger.debug(`Starting tar creation for ${compName}@${compVersion}`); | ||
| const archiveBuffer = await createTarInMemory(tempDir); |
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.
should this try-catch the error generated in the function?
extensions/archive-attachments.js
Outdated
| for (const attachment of matched) { | ||
| const relPath = attachment.out.path; | ||
| // Include only the part of the path after '_attachments/' | ||
| const attachmentsSegment = '_attachments/'; |
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.
if const, consider creating outside of the for-loop and define at the start of the script
extensions/archive-attachments.js
Outdated
| const attachmentsIndex = relPath.indexOf(attachmentsSegment); | ||
|
|
||
| if (attachmentsIndex === -1) { | ||
| logger.warn(`'_attachments/' segment not found in path: ${relPath}. Skipping this file.`); |
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.
use the variable instead
Deflaimun
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.
Really good new feature. New quickstart is looking 🔥
|
Thanks @Deflaimun ! I also implemented the |
Deflaimun
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 looks really good. thanks for implementing the changes
archive-attachmentsextension to automate the archiving of grouped attachments based on configurable file patterns.replace-attributes-in-attachmentsextension by allowing customizable file patterns and user-defined replacement rules.replace-attributes-in-attachmentsextension now requires configuration to specify which attachments should have their text replaced. Previously, the extension worked on all YAML attachments by default, which was inefficient as not all attachments required replacements.Motivation
In redpanda-data/docs#937, we introduce a new quickstart that requires users to download multiple files as attachments. To make the UX as streamlined as possible, we want to allow users to download and extract the files in a single archive. Allowing writers to define a group of attachments to use to build an archive reduces manual effort and ensures consistent packaging of related resources.
Benefits
Configuration Example
Tests
https://deploy-preview-90--docs-extensions-and-macros.netlify.app/preview/test/#attachments
Related PRs
Because this is a breaking change and we have an automation that auto-upgrades the package using
latest, we need to merge these PRs in before we release this new major version of the extension to avoid auto-upgrading to this major version: