Bug#875603: ITP: python-hvac -- Python 2/3 client for HashiCorp Vault
Hi Antoine,
> Here's a review.
Thanks for reviewing.
> * the debian/changelog has a typo in your email address, introduced in
> your latest commit. be more dilligent and review diffs before
> pushing! :) i have fixed it and pushed.
So _that's_ what it was... I got NMU warnings and just couldn't see why.
I assumed the changelog diff was simply the date being updated.
Looks like my DEBEMAIL got mangled somewhere.
Thanks for pointing that out.
> * it is customary (but not mandatory) to use the same license as
> upstream for the debian/* files. You use a BSD-3 license while
> upstream uses Apache-2... that seems like a gratitious change. not a
> blocker.
Personal preference.
I generally prefer permissive licenses and have used BSD 3-clause for all
Debian packages I made so far. I didn't know about the custom to use the same
license as upstream.
> * nice note about vault in the README.Debian. care to link against
> other WNPP bug reports there (opening them as needed)?
Good idea. I believe there is one for vault, but not pyhcl.
Unfortunately, I noticed the upstream source packages no longer include the
tox test suite (or they never did). Only the upstream master branch has them.
Adding the tests back or running the unit tests that are still included may be
more work. I'll see what I can do.
> * I haven't audited the upstream source, I assume you have taken at
> least a cursory look to make sure it's not total junk.
A very cursory one - a full audit would probably waste a lot of time.
At least there are no hidden "gems" like files with a different license or
somesuch.
> * lintian warns us about this:
>
> W: python-hvac: new-package-should-not-package-python2-module python-hvac
> W: python-hvac: multi-arch-same-package-calls-pycompile postinst:6
> W: python3-hvac: multi-arch-same-package-calls-pycompile postinst:6
>
> The latter should probably be fixed.
Yes, you are right. I noticed these yesterday, but simply couldn't make any
sense out of the documentation
(https://lintian.debian.org/tags/multi-arch-same-package-calls-pycompile.html)
on short notice. Multi-Arch is still giving me a headache...
I have fixed this by setting Architecture: all and removing Multi-Arch: same -
the package is completely architecture independent anyway.
> Otherwise things look good! I can sponsor this when you're ready - just
> change the changelog to update the release and we're good to go.
Thanks, I'll get back to you shortly.
Regards,
Gregor
Reply to: