Hi Will - Sorry for the delay!
Sprint + fosdem + 2 conferences made me not find the time earlier since we 
assigned it last week :-/

This is a bit more than a usual new review, but anything not part of it
will only be informational and non blocking.


# Things that are ok

- build rules are as clean as possible - thanks

- namespace check
  it is quite short, but not a common pattern.
  It matches the upstream project name.

- Lintian check
  Happy with the package



# non-blocking recommendations

- Build time tests are not seen, dh_auto_test seems not to realize there
are some (I see them in code), enabling that would be nice. I'm fine not
having autopkgtests as it depends on nothing, so it only changes by
itself

- It uses vendored code under thirdparty, quite a lot.
  And it does not have to, as far as I see it uses only benchmarks there.
  And it builds the benchmarks to then not use it in the tests :-/
  None of it is in the final binaries AFAICS which is good.
  But you could give yourself and the package a much better future by masking 
them out.
  Nowadays d/watch or similar allows you to automatically fetch and DFSG the 
tarball in a stripped form.
  Preventing you from picking up anything by accident.
  If you get the benchmarks enabled for testing keep them but nothing else?

- header only library
  This is uncommon and maybe worth to mention in the description of the package 
in d/control
  Furthermore please be aware while the automated checks won't catch you too 
fast that
  anything in main depending on it will still need this lib to go to main
  See line 76 ff in 
https://documentation.ubuntu.com/project/MIR/mir-reviewers-template/#mir-reviewers-template


# blocking issues

- license/copyright #1 - thirdparty

  Yeah I know lintian does not yell, but only because that was a cheap fast 
path saying "everything is MIT" :-)
  As long as you have the vendored code present (see the strong recommendation 
above to best just get rid of as much as you can) you need to declare those 
properly.
  See 
https://manpages.debian.org/unstable/devscripts/uscan.1.en.html#COPYRIGHT_FILE_EXAMPLES
 for more on that
  I'd suggest to see which in thirdparty you really need, then
  1. filter out all others
  2. test build if your assumption was right
  3. adapt debian/copyright to correctly cover those that you had to keep


- license/copyright #2 - bench/boost

  Somewhere in ./bench/boost... there are also Boost licensed files, again not 
MIT as specified
  For example
  ./bench/boost_bench/string.cpp: Boost Software License 1.0

  I do not know the project enough, if it is used for tests keep it, if it is 
used for the
  final code by mapping some in keep it even more. But if it is only a helpful 
tool for development
  yet has nothing to do with providing the function to the user as a 
distribution then reconsider this.

  Again, if this is used, please declare it properly.
  If it is not used consider filtering it out or at least declare it properly.


I'd generally recommend to run `licensecheck -r .` and ensure that you
are happy and feel you have declared everything that is mentioned (do
not be concerned that in many cases it can't detect - at least handle
those it can detect :-) )

** Changed in: Ubuntu Resolute
       Status: In Progress => Incomplete

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2130190

Title:
  [needs-packaging] emhash

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+bug/2130190/+subscriptions


-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to