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

Update panic message to be clearer about env-vars #56748

Merged
merged 1 commit into from
Dec 14, 2018

Conversation

kinnison
Copy link
Contributor

Esteban Kuber requested that the panic message make it clear
that RUST_BACKTRACE=1 is an environment variable. This change
makes that clear.

I understand that this may simply be closed if the concept isn't accepted, and I'd be fine with that :-)

Fixes #56734

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dtolnay (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. Due to 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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2018
@dtolnay
Copy link
Member

dtolnay commented Dec 13, 2018

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned dtolnay Dec 13, 2018
@dtolnay
Copy link
Member

dtolnay commented Dec 13, 2018

Thanks for the PR! Esteban will get back to you about whether this message is what they had in mind.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 13, 2018

📌 Commit f5ec9100b95e4801857641a35afde5b6e37ace45 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2018
@dtolnay
Copy link
Member

dtolnay commented Dec 13, 2018

I think it would be valuable to use the wording "environment variable" because that is way more searchable than "in the environment" for people to whom it isn't clear how to set things in the environment.

I would use the following variation which is only slightly wider than your current message.

- Run with `RUST_BACKTRACE=1` in the environment to display a backtrace.
+ Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

@estebank
Copy link
Contributor

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 13, 2018
@estebank
Copy link
Contributor

r=me after @dtolnay's change

Esteban Kuber requested that the panic message make it clear
that `RUST_BACKTRACE=1` is an environment variable.  This change
makes that clear.  Wording provided in part by David Tolnay.
@kinnison
Copy link
Contributor Author

I apologise for the delay. I agree with @dtolnay 's rewording and have amended the commit accordingly

@dtolnay
Copy link
Member

dtolnay commented Dec 13, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Dec 13, 2018

📌 Commit 6057147 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 13, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Dec 14, 2018
Update panic message to be clearer about env-vars

Esteban Kuber requested that the panic message make it clear
that `RUST_BACKTRACE=1` is an environment variable.  This change
makes that clear.

I understand that this may simply be closed if the concept isn't accepted, and I'd be fine with that :-)

Fixes rust-lang#56734
bors added a commit that referenced this pull request Dec 14, 2018
Rollup of 14 pull requests (first batch)

Successful merges:

 - #56562 (Update libc version required by rustc)
 - #56609 (Unconditionally emit the target-cpu LLVM attribute.)
 - #56637 (rustdoc: Fix local reexports of proc macros)
 - #56658 (Add non-panicking `maybe_new_parser_from_file` variant)
 - #56695 (Fix irrefutable matches on integer ranges)
 - #56699 (Use a `newtype_index!` within `Symbol`.)
 - #56702 ([self-profiler] Add column for percent of total time)
 - #56708 (Remove some unnecessary feature gates)
 - #56709 (Remove unneeded extra chars to reduce search-index size)
 - #56744 (specialize: remove Boxes used by Children::insert)
 - #56748 (Update panic message to be clearer about env-vars)
 - #56749 (x86: Add the `adx` target feature to whitelist)
 - #56756 (Disable btree pretty-printers on older gdbs)
 - #56789 (rustc: Add an unstable `simd_select_bitmask` intrinsic)

r? @ghost
@bors bors merged commit 6057147 into rust-lang:master Dec 14, 2018
@kinnison kinnison deleted the kinnison/fix-56734 branch January 9, 2019 09:08
@dtolnay dtolnay self-assigned this Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify RUST_BACKTRACE is an env var on panic
5 participants