<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 15, 2020 at 7:22 AM Richard W.M. Jones <<a href="mailto:rjones@redhat.com">rjones@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Jun 11, 2020 at 04:19:08PM -0600, alan somers wrote:<br>
> The existing Rust bindings for nbdkit aren't very idiomatic Rust, and they<br>
> are missing a lot of features. So I've rewritten them. The new bindings<br>
> aren't backwards compatible, but I doubt that's a problem. Most likely,<br>
> nobody has tried to use them yet, since the crate hasn't even published to<br>
> <a href="http://crates.io" rel="noreferrer" target="_blank">crates.io</a>. Please review the attached patch.<br>
> -Alan<br>
<br>
Thanks Alan, and sorry for the delayed reply. It just happened that<br>
this email arrived before a long weekend.<br>
<br>
First a note that it was a bit of a manual process for me to apply<br>
this because I think it was prepared using "git diff"(?) If you use<br>
"git format-patch" or (better) "git send-email" for future patches<br>
then that's much easier. However I was able to apply and review it.<br>
<br>
I have pushed it, but added a few changes which I will summarise below:<br>
<br>
* Some lines had trailing whitespace, removed. These are only<br>
allowed in POD where it is used for verbatim sections.<br>
<br>
* In VERSION docs we usually refer only to stable (even-numbered)<br>
releases, so I replaced 1.21.9 -> 1.22.<br>
<br>
* Remove plugins/rust/Cargo.toml from .gitignore.<br>
<br>
* Several newly added files were missing from EXTRA_DIST<br>
(make && make dist && make maintainer-check-extra-dist<br>
will find these in future).<br>
<br>
This will create small rebase problems for you if you've made further<br>
changes, but hopefully nothing that isn't easily fixable.<br>
<br>
Other issues:<br>
<br>
* The license removed this clause:<br>
<br>
-// * Neither the name of Red Hat nor the names of its contributors may be<br>
-// used to endorse or promote products derived from this software without<br>
-// specific prior written permission.<br>
<br>
I believe this removal simply makes the license even more<br>
permissive, so that's fine. However I will check with our legal<br>
people. Also you should add license headers to the new files<br>
plugins/rust/tests/*.rs. Essentially every file should have a<br>
license, and correct licensing is very important to us.<br>
<br>
* Although the build works (or doesn't break), make check doesn't<br>
appear to run the tests.<br>
<br>
Rich.<br></blockquote><div><br></div><div>Ok, I can make those changes. Speaking of "make check", what about CI? It would be pretty easy to add it to this project. Is there a reason you haven't done it already?</div><div>-Alan<br></div></div></div>