Bug#868378: RFS: nlohmann-json/2.1.1-1

[Christian Seiler <christian@iwakd.de>]

Hi Muri,

On 07/16/2017 10:05 PM, Muri Nicanor wrote:
> On 07/16/2017 08:47 AM, Christian Seiler wrote:
>> This will likely break builds of reverse dependencies because they
>> might not find the header anymore. Did you test all of the reverse
>> dependencies of nlohmann-json in the archive that they'll find the
>> header in the new location? If some of them don't, you should file
>> bugs against those packages (ideally with patches) that the
>> maintainers know about this change. [1]
> There are two reverse dependencies atm, usbguard and mkvtoolnix. I've
> removed the build-dependency from usbguard, because it was not needed
> anymore and i've filed bug #868573 against mkvtoolnix and attached a patch.
> 
> Not sure if that qualifies as a library transition if its only one
> package, especially as mkvtoolnix doesn't FTBFS with the new
> nlohmann-json package (because it ships its own copy os the json.hpp
> file which it falls back to if the system one is not found...)

Since mkvtoolnix doesn't immediately FTBFS with the new library
package of yours and it really is just the one package, then yes,
I would agree that you probably don't need a transition slot for
just that.

Thanks for being so quick about filing the bug against the rdep!

Apart from that: even though I can't sponsor, I did a quick
review of the packaging (I did NOT check the upstream changes to
the version already in the archive); the package looks to be in
a very good shape in general. Some minor nitpicks that you might
want to fix (although those are really minor, they can wait for
a newer version):

 - (found by check-all-the-things [1]) you use "MIT" as the
   abbreviation, instead of the recommended "Expat" for that
   license

 - the package is still on debhelper compat 9 (10 is current,
   but see the debhelper docs for information on what defaults
   changed between those versions before changing d/compat)

Otherwise: it builds fine in sbuild, is lintian clean, hardening
is enabled, the packaging looks sane.

(While looking at the build log I did stumble upon another
issue, but that's in debhelper's CMake integration not keeping
up with semi-recent CMake features. I've reported that issue
in #868584, in case you're interested.)

Anyway, hope my review helps you in getting the package
sponsored sooner.

Regards,
Christian

[1] This takes a _long_ time with this package as you have huge
    test data in JSON form within the package, and if you do
    run it, redirect its output into a file, otherwise your
    terminal will be swamped with messages.
    (Also note that check-all-the-things has some false
    positives in your case, e.g. it checks for correct JSON as
    one of its checks, and your package has some intentionally
    broken JSON as unit tests.)