-
Notifications
You must be signed in to change notification settings - Fork 8
chore(none): 🤖 update yaml config #130
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request updates several GitHub workflow definitions, adds a new pre-commit hook, and introduces TypeScript support for automating artifact updates. The changes include reformatting build commands, reorganizing artifact upload steps, dynamically generating package lists for release workflows, and adding a new script that scans plugin directories to update workflow files. No modifications were made to exported or public entities. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant PreCommit as Husky Pre-commit Hook
participant Script as update-artifacts.ts
participant FS as File System
participant WF as Build Workflow File
Dev->>PreCommit: Initiate commit
PreCommit->>Script: Run "pnpm run update-artifacts"
Script->>FS: Read directories in ./rust-plugins
FS-->>Script: Return list of plugin directories
Script->>WF: Read current .github/workflows/build.yaml
Script->>WF: Remove outdated upload steps
Script->>WF: Add new upload steps using generateUploadStep()
Script->>FS: Write updated workflow content
Script-->>PreCommit: Log processed plugin count
sequenceDiagram
participant Git as GitHub Push
participant Runner as GitHub Actions Runner
participant Workflow as Release Workflow
participant Shell as Shell Executor
Git->>Workflow: Push change to main with rust-plugins modifications
Workflow->>Workflow: Trigger release job (path filter active)
Workflow->>Shell: Execute "Get Package List" command
Shell-->>Workflow: Return formatted list of package names
Workflow->>Workflow: Iterate over packages in "Move Artifacts" step
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
🧹 Nitpick comments (9)
.github/workflows/release-rust-plugins.yml (1)
31-36: Dynamic package discovery improves maintainability.Using shell commands to automatically detect plugin directories is a good approach to eliminate hardcoding. However, the directory structure parsing is somewhat brittle.
Consider making the directory structure detection more robust:
- PACKAGES=$(ls -d ./rust-plugins/*/ | cut -d'/' -f3 | tr '\n' ' ') + PACKAGES=$(find ./rust-plugins -maxdepth 1 -mindepth 1 -type d -exec basename {} \; | tr '\n' ' ')scripts/update-artifacts.ts (4)
1-3: Update Node.js module imports to use the newer protocol.Current imports work but for newer Node.js versions, it's recommended to use the
node:protocol for built-in modules.- import * as fs from 'fs'; - import * as path from 'path'; + import * as fs from 'node:fs'; + import * as path from 'node:path'; import * as yaml from 'yaml';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
38-41: Consider using optional chaining instead of the&&operator.The current filtering logic works but can be simplified with optional chaining, and the TypeScript ignore comment could be avoided with proper typing.
- // @ts-ignore - buildJob.steps = buildJob.steps.filter(step => - !(step.name && step.name.startsWith('Upload Plugin ')) - ); + // Define a type for the step + type WorkflowStep = {name?: string; [key: string]: any}; + + buildJob.steps = buildJob.steps.filter((step: WorkflowStep) => + !step.name?.startsWith('Upload Plugin ') + );🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
44-46: Consider usingfor...ofinstead offorEachfor better performance.Using
for...ofloops can be more efficient thanforEachfor large arrays and improves readability.- plugins.forEach(plugin => { - buildJob.steps.push(generateUploadStep(plugin)); - }); + for (const plugin of plugins) { + buildJob.steps.push(generateUploadStep(plugin)); + }🧰 Tools
🪛 Biome (1.9.4)
[error] 44-46: Prefer for...of instead of forEach.
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
(lint/complexity/noForEach)
20-52: Add error handling for file operations.The script lacks error handling for file operations, which could make it more robust when dealing with file system operations.
Consider adding try/catch blocks:
function updateWorkflow() { + try { // Read the workflow file const workflowPath = path.join(process.cwd(), WORKFLOW_FILE); const workflowContent = fs.readFileSync(workflowPath, 'utf8'); const workflow = yaml.parse(workflowContent); // Get all rust plugin directories const pluginsPath = path.join(process.cwd(), RUST_PLUGINS_DIR); const plugins = fs.readdirSync(pluginsPath) .filter(name => fs.statSync(path.join(pluginsPath, name)).isDirectory()); // Find the build job const buildJob = workflow.jobs.build; if (!buildJob) { throw new Error('Could not find build job in workflow file'); } // Rest of the function... // Write back to file fs.writeFileSync(workflowPath, yaml.stringify(workflow)); console.log(`Updated workflow file with ${plugins.length} plugins`); + } catch (error) { + console.error('Error updating workflow file:', error); + process.exit(1); + } }🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 44-46: Prefer for...of instead of forEach.
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
(lint/complexity/noForEach)
package.json (1)
9-10: New Script Additions in the "scripts" Section.
The addition of the"commit": "git cz"and"update-artifacts": "ts-node scripts/update-artifacts.ts"scripts enhances your commit workflow and automates artifact updates. Please verify that the pre-commit hook (referencingpnpm run update-artifacts) is correctly integrated into your Husky configuration..github/workflows/build.yaml (3)
14-20: Updated Build Command for linux-x64-gnu.
The inline build command has been reformatted into a multi-line shell command. Please double-check that the sequence—setting the safe directory, unsetting environment variables, disabling corepack, installing pnpm, and invoking the build with the filter—is executed as intended in your CI environment. If the command grows more complex, consider refactoring this logic into an external script for maintainability.
97-99: Updated Docker Options for the Build in Docker Step.
The updated Dockeroptionsnow include volume mounts for the Cargo cache and workspace. Please verify that these volume mappings correctly point to the intended directories in your GitHub Actions environment and that there are no unexpected side effects.
103-107: Dynamic Default Build Command with Conditional Targeting.
The Default Build step constructs its command dynamically based on the matrix settings. Ensure that the conditional concatenation (usingformatand inline conditionals) behaves as expected for different configurations. Adding inline comments or documentation here might help future maintainers understand the logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.github/workflows/build.yaml(3 hunks).github/workflows/release-rust-plugins.yml(3 hunks).husky/pre-commit(1 hunks)package.json(1 hunks)scripts/update-artifacts.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
scripts/update-artifacts.ts
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 44-46: Prefer for...of instead of forEach.
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
(lint/complexity/noForEach)
🔇 Additional comments (11)
tsconfig.json (1)
1-10: Well-structured TypeScript configuration.This is a standard and appropriate TypeScript configuration for a Node.js environment with good defaults:
- ES2018 target is suitable for modern Node.js
- Strict type checking enabled for improved code quality
- CommonJS module system for Node.js compatibility
- Essential flags for better developer experience
.husky/pre-commit (1)
1-5: Good automation with pre-commit hook.This pre-commit hook ensures that workflow files are automatically updated and staged before each commit, maintaining consistency in the CI/CD configuration.
.github/workflows/release-rust-plugins.yml (2)
6-7: Appropriate path filter addition.Adding path filters to only trigger the workflow when files in the
rust-pluginsdirectory are modified is a good optimization.
46-46: Dynamic package iteration.Using the dynamically generated package list in the loop is a good improvement over hardcoded values.
scripts/update-artifacts.ts (1)
8-18: Well-structured upload step generation function.The
generateUploadStepfunction creates a properly configured artifact upload step with appropriate parameters and theif-no-files-found: 'ignore'option is a good choice to handle missing artifacts gracefully.package.json (1)
18-25: New DevDependencies for TypeScript Support.
The newly added devDependencies (@types/node,ts-node,typescript, andyaml) align well with the introduction of the artifact update script. Ensure that the specified versions are compatible with your project setup and that your team is aware of the new TypeScript-based tooling..github/workflows/build.yaml (5)
24-27: Reformatted Build Command for linux-x64-musl.
Similar to the linux-x64-gnu configuration, the build command here uses multi-line formatting. Verify that unsetting the environment variable (CC_x86_64_unknown_linux_musl) and the subsequent pnpm invocation operate reliably across your build agents.
37-44: Windows Build Command for win32-ia32-msvc.
The usage of a folded scalar (>) improves readability for the Windows build command. Please ensure that the exported environment variables and the sequence of installingcargo-xwinand invoking pnpm work as expected on the targeted Windows environment.
48-57: Windows Build Command for win32-arm64-msvc.
This build command also uses a folded scalar to maintain clarity. Double-check that the exports (like settingCARGO_PROFILE_RELEASE_CODEGEN_UNITSandCARGO_PROFILE_RELEASE_LTO) and the subsequent commands, including the installation ofcargo-xwin, execute correctly for the arm64 target.
90-91: Explicit Zig Version Specification.
Specifying Zig version0.13.0in the setup step is clear and helps ensure consistency. Confirm that this version meets your project’s requirements and works harmoniously with your build process.
113-202:Details
❓ Verification inconclusive
Artifact Upload Steps Review and Consistency Check.
The workflow now includes a comprehensive set of artifact upload steps for various Rust plugins. Notably, the new"Upload Plugin yaml"step has been added (lines 197–202) to handle YAML artifacts, which is in line with the updated automation. However, according to the AI summary, the"Upload Plugin mdx"(lines 143–148) and"Upload Plugin modular-import"(lines 149–154) steps were expected to be removed and replaced by this new step. Since both sets of steps still exist, please confirm whether the mdx and modular-import steps are still required or if they should be removed to avoid redundancy and maintain consistency with your planned workflow improvements.
Artifact Upload Steps Consistency: Clarification Needed
The workflow now includes both the legacy "Upload Plugin mdx" (lines 143–148) and "Upload Plugin modular-import" (lines 149–154) steps, alongside the new "Upload Plugin yaml" (lines 197–202) step. Previously, the mdx and modular-import steps were expected to be removed in favor of the streamlined YAML upload. Please confirm if these mdx and modular-import steps are still required or if they should be removed to avoid redundancy and maintain consistency with the planned workflow improvements.
Summary by CodeRabbit
New Features
Refactor
Chores