search

alexcrichton / filetime

Accessing file timestamps in a platform-agnostic fashion in Rust
Apache License 2.0
122 stars 56 forks source link

Use cvt to simplify syscall error handling #46

Closed marmistrz closed 4 years ago

marmistrz commented 4 years ago

The CI failure is unrelated to this change.

alexcrichton commented 4 years ago

Thanks for the PR! I think though I'd prefer to avoid picking up a crates.io dependency for this, could the function be inlined into this crate?

marmistrz commented 4 years ago

Basically, I created the cvt precisely to be able to share this small piece of code between projects. We're already using it in CraneStation/wasi-common (precisely in the winx subcrate so far) and this dependency (however small) would be reused if we also used the crate here.

alexcrichton commented 4 years ago

Makes sense yeah, and seems reasonable! I actually also wrote this support in the standard library :)

I would prefer to not pick up a dependency on a crate, though. I'm ok with inlining a helper function here, but if this is just to add a crate dependency I think it's ok to close this.

marmistrz commented 4 years ago

If we're not delegating the functionality to a dedicated crate, then I'm unsure of such a PR and I'll basically be indifferent to this idea and we can well close it.

alexcrichton commented 4 years ago

Ok, in that case I'm gonna go ahead and close because I'd prefer to not pick up a dependency for this.