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