Re: [libvirt-rust PATCH v3 0/4] Map more functions in stream module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux