https://bugzilla.redhat.com/show_bug.cgi?id=2120418 --- Comment #1 from nsella@xxxxxxxxxx --- Hello, thanks for your review request. I summarized some considerations that I would like to discuss with you. Most of them are not blockers and only require some clarification about the decisions made to package d-spy. Rpmlint warnings detected (after build and install): $ rpmlint -i d-spy //usr/share/appdata/org.gnome.dspy.appdata.xml: OK =================================================================== rpmlint session starts =================================================================== rpmlint: 2.2.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/licenses.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 1 d-spy.x86_64: W: no-manual-page-for-binary d-spy d-spy.x86_64: W: invalid-license GPL-3.0-or-later ==================================== 1 packages and 0 specfiles checked; 0 errors, 2 warnings, 0 badness; has taken 0.7 s ==================================== 1. W: May I ask why are the man pages absent? 2. W: About licenses. SPDX Expressions are fine, but rpmlint is complaining with a warning. I suggest correcting the license with GPLv3+, and, eventually, LGPLv2+ and LGPLv3+, just for the sake of clearing warnings. Other considerations: - Why is the pkgname `d-spy` and downloadURL/tarball/GNOME name is `dspy`? This is NOT a blocker, but I noticed what could be a small inconsistency. - Regarding d-spy-libs: it is my preference to include all license files under %license. In your scenario, I would consider doing: %license COPYING.lgplv2 COPYING.lgplv3 A second option, would be putting all license files in a separate dir (LICENSE) and simply specify the dir with: %license LICENSE Again, NOT a blocker. Just my preference. - desktop file: - Quoting[^1]: "Packages containing GUI applications must include a %{name}.desktop file". This is a gray zone, but I believe it should be fixed by changing the desktop file name to d-spy.desktop. - I would use BuildRequires: desktop-file-utils[^2] instead of BuildRequires: /usr/bin/desktop-file-validate - May I ask why the installation part of the desktop file (desktop-file-install) is skipped? Is it included in the installation script? In that case, that's fine. - appdata: - I would use BuildRequires: libappstream-glib[^3] instead of BuildRequires: /usr/bin/appstream-util - Usually, subpackages other than devel should require the base package using a fully versioned dependency.[^4] This is not the case for all packages, I just wanted to point this out to be sure it is not a mistake. Is it intentional? Here are all my comments, feel free to disagree and discuss my suggestions. Overall, the package is good. It builds and installs fine! Nicola [^1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/ReviewGuidelines/ [^2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_file_install_usage [^3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/#_app_data_validate_usage [^4]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package -- 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=2120418 _______________________________________________ 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
