https://bugzilla.redhat.com/show_bug.cgi?id=2075850
--- Comment #9 from Marco Rizzi <mar...@fb.com> --- Sorry Davide for the long delay, I made some changes, let's give fedora-review another run to see what's still pending. Spec URL: https://download.copr.fedorainfracloud.org/results/marcor/zeek/fedora-rawhide-x86_64/04366466-zeek/zeek.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/marcor/zeek/fedora-rawhide-x86_64/04366466-zeek/zeek-4.2.0-1.fc37.src.rpm Here are my comments: > - No %config files under /usr. > Note: %config /usr/share/zeek/site/local.zeek > [!]: %config files are marked noreplace or the reason is justified. > Note: No (noreplace) in %config /usr/share/zeek/site/local.zeek >Is this actually a config file, meaning something the user is >supposed/expected to edit? If it is, we should figure out if we can move it >under /etc (and mark it as %config(noreplace)); if it isn't, it shouldn't be >marked as %confg Looking at the zeek documentation, this is classified as a script file, so I have removed the %config reference: https://docs.zeek.org/en/master/quickstart.html#filesystem-walkthrough >- Large documentation must go in a -doc subpackage. > >I think this is a false positive? The only actual documentation I see is three >text files, that doesn't warrant a dedicated package IMO. I have removed the "CHANGES" text file from the %doc statement, that file alone is more than 1MB. >[!]: License field in the package spec file matches the actual license. >[!]: Package contains no bundled libraries without FPC exception. >These two are related; basically this thing is bundling a bunch of external >libraries like rapidjson, which is generally frowned upon. Long term, we >should figure out if we can unbundle these, but for now please add bundled() >provides for them per >https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling and >updated the License: field accordingly. I have added "bundled()" for the short term >[!]: Package requires other packages for directories it uses. > Note: No known owner of /usr/lib64/zeek, /var/log/zeek >[!]: Package must own all directories that it creates. > Note: Directories without known owners: /usr/lib/sysusers.d, > /var/log/zeek, /usr/lib64/zeek > >Add %dir entries for /var/log/zeek and /usr/lib64/zeek and add a Requires: >systemd for /usr/lib/sysusers.d done that >[!]: Patches link to upstream bugs/comments/lists or are otherwise > justified. > >Add a comment before the patch with the URL it comes from done >[-]: %check is present and all tests pass. > >Not a blocker, but if this package has tests it can run at build time, it'd be >good to add a %check section to the spec (for cmake-based projects, usually >it's enough to call %ctest if they're rigged up properly). I don't think I see tests that can run at build time. >[!]: Large data in /usr/share should live in a noarch subpackage if package > is arched. > >Consider making a zeek-data noarch subpackage for the stuff under >/usr/share/zeek I'm not sure if that's going to be possible, different packages write to /usr/share/zeek/ -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2075850 _______________________________________________ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
