heroku / libcnb.rs

A framework for writing Cloud Native Buildpacks in Rust
BSD 3-Clause "New" or "Revised" License
34 stars 6 forks source link

Consider renaming `MappedWrite::unwrap` to `MappedWrite::into_inner` #781

Open edmorley opened 6 months ago

edmorley commented 6 months ago

Whilst reviewing #721, I found initially thought the unwrap() in this usage was an unwrap() in the sense of Result::unwrap() and something that could panic:

            state: state::Section {
                write: self.state.write.unwrap(),
            },

Instead, it seems MappedWrite::unwrap in fact returns the inner writer, and can never panic.

Searching around for whether there was prior art we could base the naming on (returning some inner field), I found that BufWriter actually has an into_inner which does exactly what we're doing in MappedWrite::unwrap: https://doc.rust-lang.org/std/io/struct.BufWriter.html#method.into_inner

As such, renaming to MappedWrite::into_inner seems to make sense to me?

@Malax Thoughts?