-
Notifications
You must be signed in to change notification settings - Fork 357
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: [M3-7274] - Update curl-bash with system default /bin/bash #11021
Conversation
Coverage Report: ✅ |
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.
Does this change actually address the issue raised by M3-7274?
With all the contributions we've been getting from outside of our team, I'm wondering if it might make sense to get rid of getting_started.sh
altogether and encourage users to install tools and other dependencies following the step-by-step instructions:
- It really doesn't take very much effort to install the needed tools manually
getting_started.sh
was written with Mac users in mind, so we'll have to support manual installation regardless if we ever have contributors on Linux- It's pretty heavy handed and opinionated in terms of what it installs -- that makes sense for our team where we have consensus, but if I were a random contributor and ran this script and it unexpectedly installed Homebrew, Volta, GitHub CLI, etc., I'd be annoyed. To pick on Volta as an example, it's not even required for development and just happens to be our blessed version manager -- it would be just as valid for a contributor to install Node 20 directly or use a version manager that they already have installed
That's a good point @jdamore-linode . I don't know if this would fix the unsafe piping that this describes I'm cool with removing |
Changes 🔄
/bin/bash -c
is more explicit and ensures we're using the system's default bash located at/bin/bash
. It aligns with HomeBrews recommended method on installation. https://brew.sh/Note
I did not think we needed hash verification for this, but we can if we wanted to make this more of a manual step for users and increase the size of this script by adding a shasum verification step. Open to opinions.Target release date 🗓️
N/A
As an Author I have considered 🤔
Check all that apply