Re: [libvirt-rust PATCH v3 0/4] Map more functions in stream module
On Wed, Jan 29, 2020 at 08:24:10PM -0700, Zixing Liu wrote:
This set of patches will add more functions to the Rust bindings.
Newly mapped functions from C library: virStreamNew virStreamEventUpdateCallback
virStreamEventRemoveCallback virStreamEventAddCallback.
virStreamEventAddCallback can accept normal fn functions or closures (can capture
variables outside)
The changes are not very thoroughly tested since event module is not implemented at all
so the virStreamEventAddCallback will always return "unsupported by the connection
driver".
Version 2: Addressed comments
Version 3: Undo format changes and rebased against latest branch
The version 3 of your patches don't pass CI. Please consider to use
`cargo fmt -v -- --check` to validate the coding style before to
submit any patches. Also you can use `cargo fmt` to help you for the
coding style.
Zixing Liu (4):
libvirt-rust: stream: add more functions in stream
libvirt-rust: stream: add more functions in stream
I would imagine both of these patches can be merged together, `git
rebase -i master` and usage of `squash` is really helpful of that.
For the title you should find something a bit more explicit like:
stream: add better coverage for the stream API
libvirt-rust: use reference instead of moving
libvirt-rust: stream: addressed comments
The best is to have the comments addressed in the patch itself. This
can easily be achieved using `git rebase -i master` and by editing the
right patch.
Besides that your patches are OK. If you don't mind I could take care
of addressing my comments before to merge them. You don't have to be
worry, you still keep the credits for them.
Sounds good for you?
src/domain.rs | 2 +-
src/stream.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++++---
tests/stream.rs | 40 +++++++++++++++++++++
3 files changed, 130 insertions(+), 6 deletions(-)
create mode 100644 tests/stream.rs
--
2.25.0