waku-org / waku-rust-bindings

Rust wrapper over go-waku ffi
14 stars 6 forks source link

feat: example app and `waku_destroy` #99

Closed richard-ramos closed 4 months ago

richard-ramos commented 4 months ago

qq: why waku_destroy is needed? We don't have such concept in nwaku-libwaku.

The reason why it is necessary is because in waku_new we are creating a context running its own thread, yet we're not stopping that thread nor releasing the resources allocated by the Context by calling stopWakuNodeThread .

In https://github.com/waku-org/nwaku/pull/2499 I added the waku_destroy function which calls that function, so we can do a proper cleanup of the resources used by waku when we no longer require a node. (Think of it as a graceful shutdown)

Ivansete-status commented 4 months ago

qq: why waku_destroy is needed? We don't have such concept in nwaku-libwaku.

The reason why it is necessary is because in waku_new we are creating a context running its own thread, yet we're not stopping that thread nor releasing the resources allocated by the Context by calling stopWakuNodeThread .

In waku-org/nwaku#2499 I added the waku_destroy function which calls that function, so we can do a proper cleanup of the resources used by waku when we no longer require a node. (Think of it as a graceful shutdown)

Wonderful! Thanks for that!

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 15.07937% with 107 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (nwaku@69a4872). Click here to learn what that means.

Files Patch % Lines
examples/basic/src/main.rs 0.00% 107 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## nwaku #99 +/- ## ======================================== Coverage ? 70.00% ======================================== Files ? 11 Lines ? 860 Branches ? 0 ======================================== Hits ? 602 Misses ? 258 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.