Skip to content

Reorganize, warn ab. panic=abort with test/bench #4075

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 4 commits into from
Closed

Reorganize, warn ab. panic=abort with test/bench #4075

wants to merge 4 commits into from

Conversation

sanmai-NL
Copy link

@sanmai-NL sanmai-NL commented May 20, 2017

This PR resolves #3166 (comment) a bit more thoroughly.

  • Was partially using information hiding by using nested functions. Apply this more consistently.
  • Add warning when panic runtime parameter shall be ignored since set in test or bench profile.

The new stuff is only a few lines down here master...sanmai-NL:Reorganize_toml.rs_warn_about_panic_runtime#diff-f4550ee862e63b97b52a09a7f644ac7bR651 .

* The file is partially using information hiding by using nested functions.
  Apply this more consistently.
* Add warning when panic runtime parameter shall be ignored since set in test
  or bench profile.
@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@sanmai-NL
Copy link
Author

I have tested locally using --package cargo --test test test_release_ignore_panic but even though standard error is supposed to be shown per my IDE settings, I did not see the warning propagate. I suppose you will test again and give more guidance if needed.

@sanmai-NL
Copy link
Author

@alexcrichton: can you restart the AppVeyor pipeline?

@alexcrichton
Copy link
Member

Thanks for the PR! Could the refactorings be left to a separate commit or perhaps a separate PR?

@alexcrichton
Copy link
Member

Closing due to inactivity, but please feel free to resubmit with the refactorings extracted!

@sanmai-NL sanmai-NL deleted the Reorganize_toml.rs_warn_about_panic_runtime branch February 17, 2018 16:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot run benchmarks with panic = "abort"
3 participants