Replace close with the Drop trait

radareorg / r2pipe.rs

Rust crate for r2pipe

Other

45 stars 19 forks source link

Replace close with the Drop trait #57

Closed RHL120 closed 1 year ago

RHL120 commented 1 year ago

Checklist

[ ] Closing issues: #issue
[X] Mark this if you consider it ready to merge
[ ] I've added tests (optional)
[ ] I wrote some documentation

Description I rarely implement the drop trait so I am not sure that this is the way it should be used.

trufae commented 1 year ago

@meme can you check this pr?

meme commented 1 year ago

I'm not sure about this - simply because a Drop implementation blocking on wait sounds like a bad idea. Regardless, close should not be removed - users should still be able to call close.

RHL120 commented 1 year ago

I'm not sure about this - simply because a Drop implementation blocking on wait sounds like a bad idea. Regardless, close should not be removed - users should still be able to call close.

It is clear to me now that this PR is a mistake because it will literally break all projects using r2pipe. Sadly keeping close will actually make it worse because we would then have double closes everywhere. I will close this PR. Sorry for wasting your time.