https://bugzilla.redhat.com/show_bug.cgi?id=1585758
--- Comment #4 from Petr Špaček <petr.spa...@nic.cz> --- (In reply to Jani Juhani Sinervo from comment #2) > Here's a preliminary (unofficial) review. Some points I thought would be > useful: Thank you for your time! > - Even though this is a development package by itself, and thus as far as I > can tell an unversioned .so-file is acceptable, I would still try to make > the _cqueues.so into a versioned .so-file, and then maybe creating an > symbolic link from that versioned file to _cqueues.so. As others said, this is library in private directory and it is Lua library after all, so I would prefer to avoid hassle with so versions. > - Like I've marked below, this version of the library isn't the latest. Is > there a reason for this? If there is nothing blocking you updating the > library version packaged, I would suggest making the version packaged the > latest stable release. Unfortunatelly this is the newest tarball available. I have asked upstream for latest tarball here https://github.com/wahern/cqueues/issues/202, let's see what happens. > - In your %files-section of the doc-subpackage you don't own the package > directories correctly. I'd suggest adding %{_pkgdocdir} with %dir-specifier. > Or even better, I'd add the docs using the %doc-specifier. Thanks, I will will into it! Please see below for details. > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > [ ] = Manual review needed [...] > ===== MUST items ===== > [!]: Package requires other packages for directories it uses. > Note: No known owner of /usr/share/doc/lua-cqueues/examples, > /usr/share/doc/lua-cqueues > [!]: Package must own all directories that it creates. > Note: Directories without known owners: /usr/share/lua/5.1, > /usr/share/lua/5.3, /usr/lib64/lua, /usr/share/doc/lua- > cqueues/examples, /usr/share/lua, /usr/lib64/lua/5.3, > /usr/lib64/lua/5.1, /usr/share/doc/lua-cqueues /usr/share/doc/lua-cqueues is certainly a bug, I'm not sure about others. > [!]: Each %files section contains %defattr if rpm < 4.4 > Note: %defattr present but not needed CentOS 7 has RPM 4.11 so this should not be necessary in all sections, I'm not going to extend EPEL for RHEL 6. > [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the > beginning of %install. -- Not sure if artefact from this SPEC also > seeming to work for RHEL as well Given comment#3 I'm not sure if it is really necessary. > [!]: Spec use %global instead of %define unless justified. > Note: %define requiring justification: %define luacompatver 5.1, > %define luacompatlibdir %{_libdir}/lua/%{luacompatver}, %define > luacompatpkgdir %{_datadir}/lua/%{luacompatver} I will look into this as well. -- 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 _______________________________________________ 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://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/message/WWYBW6NTRZZFIHD4V6OEY5ZVHQBLLYYK/
