faern / oneshot

Oneshot Rust channel working both in and between sync and async environments
Apache License 2.0
80 stars 9 forks source link

Improve unsafe hygiene #18

Closed Cassy343 closed 2 years ago

Cassy343 commented 2 years ago

Should contain all of, and only the desired changes discussed in https://github.com/faern/oneshot/pull/17. If there are any additional changes desired, I'll do them on this branch and deal with merge/rebase conflicts after this is merged.

faern commented 2 years ago

Awesome! I think everything is addressed now and ready for merge. I'll have another look at the code, and hopefully merge a bit later today.

faern commented 2 years ago

@Cassy343 Do you want to clean up/rewrite the git history? Otherwise I'll merge this after I've had some food :)

Cassy343 commented 2 years ago

You can go ahead and squash and merge, I always worry I'll mess something up when I edit git history so I'd rather not haha.

faern commented 2 years ago

Thanks for the contribution! Please come with feedback on the changelog entries I added for this: https://github.com/faern/oneshot/commit/3b27d35104845d9c76fabd6be1859c56377b57f3.

Looking forward to the remaining fixes and improvements :)

Cassy343 commented 2 years ago

The changelog entries look good to me!

faern commented 2 years ago

Please have a look at this comment: https://github.com/faern/oneshot/pull/18/files#r922725443. Am I missing something here, or is the test not testing what it's supposed to test?