search

streambed / streambed-rs

Event driven services toolkit
Apache License 2.0
31 stars 5 forks source link

FileLog::new should take an Into<PathBuf> #16

Closed hseeberger closed 1 year ago

hseeberger commented 1 year ago

This would allow for something like FileLog::new("foo").

Happy to come up with a PR ...

huntc commented 1 year ago

Thanks. I wonder if you’d need it though? Paths are hardly ever literals.

hseeberger commented 1 year ago

Not necessarily literals, but what about strings, e.g. from config files?

huntc commented 1 year ago

Just checked in std - the pattern appears to be using`AsRef<Path>, but here in particular, we're looking to own the path. When looking to own something, the style is to avoid passing in a ref...

Also, when reading in from config files, you'll end up with strings, not literals.

hseeberger commented 1 year ago

As I wrote, Into<PathBuf> also works for Strings, because there's an From<String> impl for PathBuf.

huntc commented 1 year ago

As I wrote, Into<PathBuf> also works for Strings, because there's an From<String> impl for PathBuf.

I really should read what you write more thoroughly! ;-)

hseeberger commented 1 year ago

I really should read what you write more thoroughly! ;-)

LOL

hseeberger commented 1 year ago

Fix in https://github.com/streambed/streambed-rs/pull/19