cloudflare / foundations

Cloudflare's Rust service foundations library.
https://blog.cloudflare.com/introducing-foundations-our-open-source-rust-service-foundation-library
BSD 3-Clause "New" or "Revised" License
1.3k stars 55 forks source link

ZTC-1478: rework PR #47 in light of the fact that the next release we do is going to have to be a major version bump anyway, no matter what #52

Closed bcottrell8738 closed 3 months ago

bcottrell8738 commented 4 months ago

I did PR #47 in the way that I did it, because I was trying to keep the externally visible API the same. There was a public function, slog_logger(), which promised to return an Arc<RwLock<Logger>>. So that meant we had to have an Arc<RwLock<Logger>> lying around. This constrained the design and made things grosser than they might otherwise have been.

It has been pointed out that we're already in the situation where the next release we do is going to need to be a major version bump, anyway. So we might as well revisit this, and do it the less-gross way that (slightly) breaks API compatibility.

We can minimize the extent to which we break API compatibility by having slog_logger() return a Arc<RwLock<impl Deref<Target = Logger>>>. This is nearly the same thing as it used to be.

This PR consists of two commits; one that reverts PR #47, and one that puts the functionality back in the new way. If you are reviewing it, you can either review the PR as a whole (which will show you the diff vs current main branch) or you can review only the second of the two commits (which will show you the diff vs how things looked prior to PR #47). I have tried to keep both sets of aesthetic considerations in mind while preparing this PR.