[PR #274] [CLOSED] Fix/vendor node output #198

New Issue

adam · 2025-12-29T08:32:01+01:00

adam commented

2025-12-29 08:32:01 +01:00

📋 Pull Request Information

Original PR: https://github.com/mountain-loop/yaak/pull/274
Author: @SyedaAnshrahGillani
Created: 10/22/2025
Status: ❌ Closed

Base: main ← Head: fix/vendor-node-output

📝 Commits (2)

e8f2f37 FIX: Suppress stdout in vendor-node.cjs version check
9eb41cb FIX: Improve protoc vendoring script error handling and output

📊 Changes

2 files changed (+6 additions, -3 deletions)

View changed files

📝 scripts/vendor-node.cjs (+1 -1)
📝 scripts/vendor-protoc.cjs (+5 -2)

📄 Description

This pull request addresses two minor but impactful issues in the project's vendoring scripts
(scripts/vendor-node.cjs and scripts/vendor-protoc.cjs), enhancing the robustness of the development
setup and improving the overall developer experience.

Changes Introduced:

scripts/vendor-node.cjs - Suppress node --version output:
- Problem: Previously, the tryExecSync function used stdio: 'inherit' when checking the vendored
  Node.js version. This caused the node --version output to be printed to the console every time
  the bootstrap script ran this check, leading to unnecessary console noise.
- Fix: Changed stdio: 'inherit' to stdio: 'pipe' in tryExecSync.
- Benefit: This change prevents extraneous output, resulting in a cleaner and less verbose console
  during the bootstrap process. Developers will now only see relevant messages, improving clarity
  and focus.
scripts/vendor-protoc.cjs - Improve error handling and suppress protoc --version output:
- Problem 1 (Error Handling): The main asynchronous function's error handler only logged errors
  but did not terminate the script with process.exit(1). If protoc vendoring failed, the bootstrap
  process would continue, potentially leading to confusing cascading errors later on.
- Problem 2 (Console Noise): Similar to vendor-node.cjs, the tryExecSync function used stdio:
  'inherit' for the protoc --version check, adding unnecessary output to the console.
- Fix 1: Added process.exit(1) to the error handler of the main async function.
- Fix 2: Changed stdio: 'inherit' to stdio: 'pipe' in tryExecSync.
- Benefit: The improved error handling ensures that the bootstrap process fails fast and provides
  immediate, clear feedback if protoc vendoring encounters an issue. This prevents wasted time
  debugging subsequent failures. The stdio: 'pipe' change also contributes to a cleaner console
  output, consistent with the vendor-node.cjs fix.

These changes are bug fixes that enhance the reliability and user-friendability of the project's
development setup, aligning with the project's contribution policy.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/mountain-loop/yaak/pull/274 **Author:** [@SyedaAnshrahGillani](https://github.com/SyedaAnshrahGillani) **Created:** 10/22/2025 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/vendor-node-output` --- ### 📝 Commits (2) - [`e8f2f37`](https://github.com/mountain-loop/yaak/commit/e8f2f372c9ba3f806dfb477179e2e3a3c509959a) FIX: Suppress stdout in vendor-node.cjs version check - [`9eb41cb`](https://github.com/mountain-loop/yaak/commit/9eb41cbf1074142623b597fce6e51715612aca8f) FIX: Improve protoc vendoring script error handling and output ### 📊 Changes **2 files changed** (+6 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `scripts/vendor-node.cjs` (+1 -1) 📝 `scripts/vendor-protoc.cjs` (+5 -2) </details> ### 📄 Description This pull request addresses two minor but impactful issues in the project's vendoring scripts (scripts/vendor-node.cjs and scripts/vendor-protoc.cjs), enhancing the robustness of the development setup and improving the overall developer experience. Changes Introduced: 1. `scripts/vendor-node.cjs` - Suppress `node --version` output: * Problem: Previously, the tryExecSync function used stdio: 'inherit' when checking the vendored Node.js version. This caused the node --version output to be printed to the console every time the bootstrap script ran this check, leading to unnecessary console noise. * Fix: Changed stdio: 'inherit' to stdio: 'pipe' in tryExecSync. * Benefit: This change prevents extraneous output, resulting in a cleaner and less verbose console during the bootstrap process. Developers will now only see relevant messages, improving clarity and focus. 2. `scripts/vendor-protoc.cjs` - Improve error handling and suppress `protoc --version` output: * Problem 1 (Error Handling): The main asynchronous function's error handler only logged errors but did not terminate the script with process.exit(1). If protoc vendoring failed, the bootstrap process would continue, potentially leading to confusing cascading errors later on. * Problem 2 (Console Noise): Similar to vendor-node.cjs, the tryExecSync function used stdio: 'inherit' for the protoc --version check, adding unnecessary output to the console. * Fix 1: Added process.exit(1) to the error handler of the main async function. * Fix 2: Changed stdio: 'inherit' to stdio: 'pipe' in tryExecSync. * Benefit: The improved error handling ensures that the bootstrap process fails fast and provides immediate, clear feedback if protoc vendoring encounters an issue. This prevents wasted time debugging subsequent failures. The stdio: 'pipe' change also contributes to a cleaner console output, consistent with the vendor-node.cjs fix. These changes are bug fixes that enhance the reliability and user-friendability of the project's development setup, aligning with the project's contribution policy. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

adam added the pull-request label 2025-12-29 08:32:01 +01:00

adam closed this issue

2025-12-29 08:32:02 +01:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/yaak-mountain-loop#198