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
