Skip to content
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

fix: handle paths with spaces in getGlobalEsbuildVersion function #7571

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

dxdc
Copy link
Contributor

@dxdc dxdc commented Aug 20, 2024

Description

  • Wrapped the binPath in double quotes to correctly handle paths containing spaces.
  • Ensured execSync properly executes the command when the binary path includes spaces.

Scenarios Tested

Sample Commands

Copy link

google-cla bot commented Aug 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@joehan
Copy link
Contributor

joehan commented Aug 20, 2024

Looks like there are test failures for this, please fix.

@dxdc
Copy link
Contributor Author

dxdc commented Aug 20, 2024

done

@jamesdaniels
Copy link
Member

@leoortizz PTAL

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.90%. Comparing base (18e3189) to head (ca51c59).
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7571      +/-   ##
==========================================
- Coverage   56.91%   56.90%   -0.01%     
==========================================
  Files         396      396              
  Lines       27599    27599              
  Branches     5692     5692              
==========================================
- Hits        15708    15706       -2     
- Misses      11723    11725       +2     
  Partials      168      168              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@chalosalvador chalosalvador left a comment

Choose a reason for hiding this comment

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

LGTM!

@leoortizz leoortizz merged commit 6bd497d into firebase:master Sep 18, 2024
38 checks passed
leoortizz added a commit that referenced this pull request Sep 18, 2024
leoortizz added a commit that referenced this pull request Sep 18, 2024
joehan added a commit that referenced this pull request Sep 18, 2024
* add changelog for #7571

* Update CHANGELOG.md

---------

Co-authored-by: joehan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants