slog-atomic 0.3.0 out of date with master

[andreastt]

I suspect the slog-atomic crate was not published with the 1.0.0 release as the code on master does not match that in 0.3.0.

0.3.0:

/// Handle to `AtomicSwitch` allowing switching it's sub-drain
pub struct AtomicSwitchCtrl<E>(Arc<ArcCell<Box<Drain<Error=E>>>>);

/// Drain allowing atomically switching a sub-drain in runtime
pub struct AtomicSwitch<E>(Arc<ArcCell<Box<Drain<Error=E>>>>);

impl<E> AtomicSwitchCtrl<E> {
    /// Create new `AtomicSwitchCtrl`
    pub fn new<D: Drain<Error=E> + 'static>(d: D) -> Self {
        let a = Arc::new(ArcCell::new(Arc::new(Box::new(d) as Box<Drain<Error=E>>)));
        AtomicSwitchCtrl(a)
    }

    /// Create new `AtomicSwitchCtrl` from an existing `Arc<...>`
    pub fn new_from_arc(d: Arc<ArcCell<Box<Drain<Error=E>>>>) -> Self {
        AtomicSwitchCtrl(d)
    }

    /// Get a `AtomicSwitch` drain controlled by this `AtomicSwitchCtrl`
    pub fn drain(&self) -> AtomicSwitch<E> {
        AtomicSwitch(self.0.clone())
    }

    /// Set the drain
    pub fn set<D: Drain<Error=E>>(&self, drain: D) {
        let _ = self.0.set(Arc::new(Box::new(drain)));
    }

    /// Swap the existing drain with a new one
    pub fn swap(&self, drain: Arc<Box<Drain<Error=E>>>) -> Arc<Box<Drain<Error=E>>> {
        self.0.set(drain)
    }
}

impl<E> Drain for AtomicSwitch<E> {
    type Error = E;
    fn log(&self, info: &Record, logger_values: &OwnedKeyValueList) -> std::result::Result<(), E> {
        self.0.get().log(info, logger_values)
    }
}

master:

/// Handle to `AtomicSwitch` that controls it.
pub struct AtomicSwitchCtrl<E>(Arc<ArcCell<Box<Drain<Error=E>>>>);

/// Drain wrapping another drain, allowing atomic substitution in runtime
pub struct AtomicSwitch<E>(Arc<ArcCell<Box<Drain<Error=E>>>>);

impl<E> AtomicSwitch<E> {
    /// Wrap `drain` in `AtomicSwitch` to allow swapping it later
    ///
    /// Use `AtomicSwitch::ctrl()` to get a handle to it
    pub fn new<D: Drain<Error=E> + 'static>(drain: D) -> Self {
        AtomicSwitch::new_from_arc(Arc::new(ArcCell::new(Arc::new(Box::new(drain) as Box<Drain<Error=E>>))))
    }

    /// Create new `AtomicSwitch` from an existing `Arc<...>`
    ///
    /// See `AtomicSwitch::new()`
    pub fn new_from_arc(d: Arc<ArcCell<Box<Drain<Error=E>>>>) -> Self {
        AtomicSwitch(d)
    }

    /// Get a `AtomicSwitchCtrl` handle to control this `AtomicSwitch` drain
    pub fn ctrl(&self) -> AtomicSwitchCtrl<E> {
        AtomicSwitchCtrl(self.0.clone())
    }
}

impl<E> AtomicSwitchCtrl<E> {
    /// Get Arc to the currently wrapped drain 
    pub fn get(&self) -> Arc<Box<Drain<Error=E>>> {
        self.0.get()
    }

    /// Set the current wrapped drain
    pub fn set<D: Drain<Error=E>>(&self, drain: D) {
        let _ = self.0.set(Arc::new(Box::new(drain)));
    }

    /// Swap the existing drain with a new one
    pub fn swap(&self, drain: Arc<Box<Drain<Error=E>>>) -> Arc<Box<Drain<Error=E>>> {
        self.0.set(drain)
    }

    /// Get a `AtomicSwitch` drain controlled by this `AtomicSwitchCtrl`
    pub fn drain(&self) -> AtomicSwitch<E> {
        AtomicSwitch(self.0.clone())
    }
}

impl<E> Drain for AtomicSwitch<E> {
    type Error = E;
    fn log(&self, info: &Record, logger_values: &OwnedKeyValueList) -> std::result::Result<(), E> {
        self.0.get().log(info, logger_values)
    }
}

[dpc]

Should be fixed now. Comments always welcome. :)

[andreastt]

Thanks!

Generally I find it quite confusing with the plethora of independent crates that are not kept in sync with published documentation and examples. From a maintenance point of view, a single extern crate slog dependency would be easier to manage as figuring out what different crate versions are compatible has been a challenge.

[dpc]

I understand. Now that core slog crate is stable, every other crate should depend on slog = "1", so cargo should have easier time just picking a version for you. Right now it was just me, that did not publish atomic after the final breaking change: 1.0.0.

The reason why they are broken up in pieces is that each crate pull in more dependencies, so having them in one crate would pull a lot of dependencies. Then they are broken up, you're paying just for what you're using. Especially for libraries, that don't actually prepare any drain, that's a huge benefit, as they depend only on tiny slog core crate.

If it keeps being an inconvenience please get back to me. :) . Maybe eg. we could create slog-all crate the rexports both slog and all other (sometimes interdependent) slog crates in a one set or something. We'll see how it plays out.

[andreastt]

Thanks for your elaborate reply!

I have some theoretical appreciation for why the library is built the way it is, but I’m not convinced the need to depend on e.g. slog-atomic or slog-term independently from slog, or the minor improvements in build time, makes up for this complexity.

An alternative scheme would be to synchronise and force-push each crate’s version number when changes occur, so that when a change happens in slog-atomic, all crates are pushed to the same version, making a consumer’s Cargo.toml file easier to maintain:

slog = "^1"
sloc-atomic = "^1"
…

Under any circumstances, I expect changes to be more minor now that the library has reached a stable version number, and I don’t expect the changes in the coming versions to be as bad to adapt to. So please take these comments with a grain of salt (-:

[dpc]

Yes. I also advise to stick with published version, and use http://docs.rs to get current docs.