borntyping / rust-simple_logger

A rust logger that prints all messages with a readable output format.
https://crates.io/crates/simple_logger
MIT License
221 stars 48 forks source link

Add #[must_use] to builder methods #25

Closed cmyr closed 4 years ago

cmyr commented 4 years ago

No worries if you don't want to take this, but was easier to write the PR than to open a discussion issue.

This will warn the user if they do not call the init() method.

This patch also tweaks the docs in a few places to highlight the need to call init.

This was motiviated because I failed to do this myself when moving to the new API, and had a confused couple minutes :)

piegamesde commented 4 years ago

Alternative design proposal: If we make the builder of the logger a separate type, we can then say must_use for the whole type instead of for each method.

cmyr commented 4 years ago

@piegamesde as far as I can tell the whole SimpleLogger struct really is just a builder; you never hold onto an instance of it. An alternative solution then might be to just have #[must_use] on SimpleLogger itself? Not clear if there is any real advantage to either approach.

piegamesde commented 4 years ago

You're totally right! I'd prefer must_use on the struct itself, but in the end it does not matter that much because we can always change it.