-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
'./configure' now checks if 'config.toml' exists before writing to that destination #110123
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
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.
Can the function definition can be ommited and if os.path.isfile()
block be directly placed in if __name__ == "__main__"
?
Since there are no other function calls of the quit_if_file_exists
function and its being abstracted away unnessecerily.
Thank you for your suggestion. If it isn't against any of the style guidelines, I would like to keep it in its own function. A function has a name which goes to show intent. Also I would like to clutter as little as possible in the main function as possible. |
A comment could perhaps do the same? Though I'm unsure of it. With regard to the style guide, I believe there are no rules against abstracting away to a function but I suggested doing against that since we're dealing with python and it could increase time usage and size of the file merely 😅 I'd suggest changing the |
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.
This seems good :) r=me with the two comments addressed.
…correct exit code
Thanks! @bors r+ rollup |
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#110123 ('./configure' now checks if 'config.toml' exists before writing to that destination) - rust-lang#110429 (Spelling src bootstrap) - rust-lang#110430 (Spelling src ci) - rust-lang#110515 (Don't special-case download-rustc in `maybe_install_llvm`) - rust-lang#110521 (Fix `x test lint-docs linkchecker` when download-rustc is enabled) - rust-lang#110525 (Fix `tests/run-make-translation` when download-rustc is enabled) - rust-lang#110531 (small type system cleanup) - rust-lang#110533 (Missing blanket impl trait not public) - rust-lang#110540 (Fix wrong comment in rustc_hir/src/hir.rs) - rust-lang#110541 (Fix various configure bugs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Or just rename it as
Thanks, |
Fixes #110109
Instead of overwriting the current
config.toml
file, exit the./configure
script with a message stating why.