-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Standardize on non-zero checks for configure #27308
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
Standardize on non-zero checks for configure #27308
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
The bash scripts are honestly pretty cargo culted -- no one person truly understands them. |
I know and I plan on changing that in the future. This is just a first step to get my feet wet. |
It should be noted that if you're planning to really get into updating the bash scripts, the dream is to replace the bash scripts entirely with Cargo and rust scripts, possibly with a thin python driver for portability. CC @alexcrichton |
I feel like it's basically the case that everyone has their own dialect of bash that they write in, I don't think many take the time and truly learn everything that bash and/or generic shell scripting has to offer once something is working. As a result, as you've found, I'm sure that our configure script is quite sprawling with many coding styles and many conventions to be found throughout. It's certainly not intentional, it's largely just the first thing that works. There has been some work on the part of @cl91 to start using Cargo to build the standard library and compiler, and that would involve rewriting large portions of makefiles at least into Rust (as build scripts). It's still unclear to me how the configure script plays into all that, there's a massive amount of cargo-culted knowledge in this script and I kinda doubt that anyone really understands it all (as @gankro mentioned). Regardless this seems harmless to me, but I also would not expect it to become a hard-and-fast rule that we will strictly adhere to going into the future (we don't necessarily have a style guide for this or anything). Thanks though! |
…excrichton It's odd that "! -z" was used instead of "-n" in some places. Is perhaps, "! -z" more portable or something?
It's odd that "! -z" was used instead of "-n" in some places. Is perhaps, "! -z" more portable or something?