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

Make "backtraces" feature opt-in #157

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Aug 17, 2019

This moves the "backtraces" feature out of the default set. It also
links the Backtrace implementation to the feature, so that the
public type always exists but it does not carry any backtrace content
unless the consumer explicitely enabled that.

Closes: #113

@shepmaster shepmaster mentioned this pull request Aug 26, 2019
7 tasks
@lucab lucab force-pushed the ups/backtraces-opt-in branch 2 times, most recently from b457393 to b91fb2a Compare October 2, 2019 09:53
@lucab
Copy link
Contributor Author

lucab commented Oct 2, 2019

Rebased to latest master (and fixed a bunch of conflicts), CI is green.

@shepmaster
Copy link
Owner

My plan is to take this PR and PR #173 and ensure that they play well together locally before I merge either, then merge them separately.

Since our goal is to support the standard-library backtrace and that will be always available in some future Rust version, that's the general setup I want to mimic. I think this PR sets us up in that direction, but I don't know if it will be the final goal.

Let's assume that standard-library backtraces stabilize in Rust 1.50. My plan is that this library would grow a rust-1.50 feature flag which enables them, re-exporting it as snafu::Backtrace. Without that feature flag, Backtrace would be the inert version here. An alternate (and conflicting!) feature flag would use backtrace::Backtrace (potentially merging the backtrace-crate feature in at that point too).

@shepmaster
Copy link
Owner

I'll take care of addressing these conflicts, as part of that local testing!

@shepmaster shepmaster changed the title snafu: make "backtraces" feature opt-in Make "backtraces" feature opt-in Oct 8, 2019
@shepmaster shepmaster force-pushed the ups/backtraces-opt-in branch 2 times, most recently from 03d9ca6 to 51de49f Compare October 9, 2019 23:37
This moves the "backtraces" feature out of the default set. It also
links the `Backtrace` implementation to the feature, so that the
public type always exists but it does not carry any backtrace content
unless the consumer explicitely enabled that.
@shepmaster shepmaster merged commit 32f1e29 into shepmaster:master Nov 6, 2019
@shepmaster
Copy link
Owner

Thanks!

@shepmaster shepmaster added breaking change Likely requires a SemVer version bump enhancement New feature or request labels Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Likely requires a SemVer version bump enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please make "backtraces" feature opt-in, not opt-out
2 participants