https://bugzilla.redhat.com/show_bug.cgi?id=1658153 Jerry James <loganjerry@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |loganjerry@xxxxxxxxx --- Comment #20 from Jerry James <loganjerry@xxxxxxxxx> --- Here are some issues I see with this package: 1. Consider patching the sources to invoke xdg-open as the browser, and replace "Requires: firefox" with "Requires: xdg-utils". That lets the user specify a browser via desktop settings. 2. Why "Requires: kolourpaint" and not, say, gimp? Both are in the KNOWN_IMAGE_EDITORS list. Why pick one? If one image editor is replaceable with another, then add Suggests for each of them, allowing the user to pick the one (s)he wishes to use. 3. Why "Requires: lxterminal"? That's wrong for users who are not using LXDE. This seems similar to #2. I see that xterm, rxvt, gnome-terminal, konsole, etc. are also supported. Requiring a particular one does not seem right. Add Suggests for each if you like, but don't force one. 4. Add "Requires: coreutils" since the code invokes cat, echo, ls, mkdir, etc. It's probably silly, since who could have a system with no coreutils? Nevertheless, humor me. 5. Likewise, add "Requires: grep", since I see grep being invoked as well. 6. Please make a -doc subpackage for the documentation. It consists of 23214080 bytes in 1066 files, which counts as "large". See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation 7. There are a lot of licenses in this code. I'm not confident that GPLv3+ is an accurate summation of them. The licensecheck tool produces a lot of output, and shows (for example) that the GFDL is at play. I think a thorough license analysis needs to be done. 8. The code in vcglib/eigenlib looks like a bundled copy of eigen3. Please check whether it is or not. If it is, it needs to be unbundled. 9. Others have already told you that the %configure and desktop-file-install lines are too long. See #4 in comment 16. Fix them, please. 10. The %changelog entry still isn't right. See #5 in comment 16. Fix it, please. 11. Please just remove, rather than comment out, useless lines like this one in %install: #/usr/lib/rpm/find-debuginfo.sh and this one in %files: #/usr/lib/debug/usr/bin/dune.debug They are just clutter at this point. 12. Use %{_docdir} or %{_pkgdocdir} in the %configure invocation instead of "/usr/share/doc". 13. The dune4kids man page uses a "pp" macro that our man doesn't know about. -- 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@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxxxxx
