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

subscriber: fix missing make_writer_for in BoxMakeWriter #1695

Merged
merged 1 commit into from Oct 29, 2021

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Oct 29, 2021

Motivation

In tracing-subscriber 0.3.x, MakeWriter filtering seems to have
stopped working when using the BoxMakeWriter wrapper to erase the type
of a MakeWriter implementation. It looks like what happened is that
commit 6cc6c47, which backported the
change to add a lifetime parameter to MakeWriter (#781), I
accidentally clobbered the make_writer_for method on the inner Boxed
type, so that it only has make_writer:
6cc6c47#diff-c5dc275b15a60c1a2d4694da3797f4247c4f2e1e0978fd210dd14452d6746283L737-L739

This meant that any filtering performed by the MakeWriter inside the
box is now ignored. My bad!

Solution

This commit puts back the missing make_writer_for method. Whoops!

Fixes #1694

## Motivation

In `tracing-subscriber` 0.3.x, `MakeWriter` filtering seems to have
stopped working when using the `BoxMakeWriter` wrapper to erase the type
of a `MakeWriter` implementation. It looks like what happened is that
commit 6cc6c47, which backported the
change to add a lifetime parameter to `MakeWriter` (#781), I
accidentally clobbered the `make_writer_for` method on the inner `Boxed`
type, so that it only has `make_writer`:
6cc6c47#diff-c5dc275b15a60c1a2d4694da3797f4247c4f2e1e0978fd210dd14452d6746283L737-L739

This meant that any filtering performed by the `MakeWriter` inside the
box is now ignored. My bad!

## Solution

This commit puts back the missing `make_writer_for` method. Whoops!

Fixes #1694
@hawkw hawkw requested a review from a team as a code owner October 29, 2021 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants