https://bugzilla.redhat.com/show_bug.cgi?id=2315886 --- Comment #5 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- (In reply to Sandro from comment #3) > > Then, to be strictly correct: > > > > # include/valijson/compat/optional.hpp is derived/forked from an unknown > > # release of the Boost.Optional library > > Provides: bundled(boost) > > This feels very strict seeing this is concerning a single header file. Is it > even true? The package is not providing any binaries, nor libraries, etc.? > I'd rather not add the Provides. I'd be happy to add a comment regarding > that file, though. I tend to be stricter than most on indicating bundling. When considering whether it’s worth considering forked or “inspired by” code as bundling, I tend to ask myself, “If a serious bug were discovered in the original code, how likely is it that this code would also be affected?” But there is room for nuance and subjective judgement here. In general, a single copied header file can certainly constitute bundling. I would tend not to consider it bundling if the header is mostly function signatures, but Boost.Optional is a header-only (sub)library within boost. The contents of this header file are nontrivial, and are compiled into anything that uses the header-only library in a C++ standard mode earlier than C++17. The only differences between this and including a massive amalgamated single-file header-only library like nlohmann_json.hpp are (1) scale, and (2) the degree of copying from the original implementation. On second look, this second point – the degree of copying from the original implementation – does appear persuasive in this case. The header comment says “The idea and interface is based on Boost.Optional library,” and it seems fair to say that it appears more “inspired by” than “copied from” Boost.Optional. I think this is a good argument that it’s not necessary to indicate bundling in this case. In any case, it looks like you’ve accounted for the license in this file, which is the really necessary part. (In reply to Sandro from comment #4) > Spec URL: https://gui1ty.fedorapeople.org/review/valijson.spec > SRPM URL: > https://gui1ty.fedorapeople.org/review/valijson-1.0.3-8.fc42.src.rpm > > Changes have been implemented as per above comments. This release also > "fixes" the tests. It turned out the release of the test suite was too far > ahead of what upstream is using. I'm now using the exact same version as > upstream. Thanks! I’m glad you were able to figure out the issue with the tests. I’ll revisit this review as soon as I have a little time. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2315886 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202315886%23c5 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
