Skip to content

replace shell scripts with node#10

Open
Shane98c wants to merge 2 commits intomainfrom
scripts-to-node
Open

replace shell scripts with node#10
Shane98c wants to merge 2 commits intomainfrom
scripts-to-node

Conversation

@Shane98c
Copy link
Copy Markdown
Collaborator

@Shane98c Shane98c commented Apr 2, 2026

Any windows devs who want to help out will appreciate this!

  • Replaced 3 .sh scripts (sync-fixtures, sync-flatbuffers, ensure-flatc) with cross-platform Node.js (.mjs)
  • Consolidated both sync scripts into a single sync.mjs with --if-missing and --branch flags
  • Extracted inline shell pipelines (generate:fbs, check:fbs, clean) into proper scripts
  • ensure-flatc.mjs now supports Windows (PowerShell download + .exe binary)
  • All package.json scripts now follow a consistent node scripts/... pattern

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request migrates several shell scripts to Node.js, improving cross-platform compatibility and maintainability. The review identified several security and robustness concerns, including command injection vulnerabilities in scripts/sync.mjs due to shell interpolation, missing validation for the --branch argument, and potential issues with unquoted paths in git sparse-checkout commands. Additionally, it was suggested to improve the robustness of PowerShell path handling in scripts/ensure-flatc.mjs to prevent failures with special characters.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant