https://bugzilla.redhat.com/show_bug.cgi?id=2068693 Gustavo Costa <xfgusta@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@xxxxxxxxxxxxxxxxx |xfgusta@xxxxxxxxx Doc Type|--- |If docs needed, set a value CC| |xfgusta@xxxxxxxxx Flags| |fedora-review? | |needinfo?(justin.zobel@gmai | |l.com) --- Comment #1 from Gustavo Costa <xfgusta@xxxxxxxxx> --- Hi Justin. This is my first attempt to do an official review, if you're not happy with something let me know. Some changes I will propose are to improve the maintainability of the spec. You can see all the changes I made here: https://xfgusta.fedorapeople.org/review/qview.spec. --- Define an appid --- It's useful to define the application ID as we will be using it a few times. > %global appid com.interversehq.qView --- Use URL instead of Url --- I can't really tell you if "Url" is valid, but most packages use "URL". Pagure, for instance, doesn't even highlight "Url". > URL: https://interversehq.com/qview --- SourceURL --- I recommend you to use the following SourceURL [1], because it makes it easier for you to bump the version of the package. > Source0: https://github.com/jurplel/%{name}/archive/%{version}/%{name}-%{version}.tar.gz You will need to update the %autosetup as well: > %autosetup -n qView-%{version} --- BuildRequires make --- Since you are using the make build system, you need to add it as a build dependency. > BuildRequires: make --- Fix unowned directory --- qView installs its icons in the /usr/share/icons/hicolor directory, which is provided by hicolor-icon-theme [2] > Requires: hicolor-icon-theme --- Use kf5-kimageformats instead of kimageformats --- It should be kf5-kimageformats instead of kimageformats [3] > Requires: kf5-kimageformats --- Use qt5-qtimageformats instead of qt5-imageformats --- It should be qt5-qtimageformats instead of qt5-imageformats [4] > Requires: qt5-qtimageformats -- Use a better description --- I think the description used in the flatpak version [5] is better than copying the summary. I would recommend it: > %description > qView is a Qt image viewer designed with minimalism and usability in mind. It > is designed to get out of your way and let you view your image without excess > GUI elements, while also being flexible enough for everyday use. --- Check the AppData and Desktop Entry --- You must validate the metainfo file using appstream-util [6]. You can also use desktop-file-validate to check the desktop entry file and install it [7]. > BuildRequires: libappstream-glib > %check > appstream-util validate-relax --nonet %{buildroot}/%{_metainfodir}/%{appid}.appdata.xml > desktop-file-validate %{buildroot}/%{_datadir}/applications/%{appid}.desktop Also, remove the desktop-file-install in the %install section. --- Add a doc. file --- The README.md file contains some relevant information, such as a link to the changelog [8] > %doc README.md --- Use the appid and globs --- > %{_datadir}/applications/%{appid}.desktop > %{_metainfodir}/%{appid}.appdata.xml You can also use glob here > %{_datadir}/icons/hicolor/*/apps/* References: 1. https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags 2. https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories 3. https://src.fedoraproject.org/rpms/kf5-kimageformats 4. https://src.fedoraproject.org/rpms/qt5-qtimageformats 5. https://flathub.org/apps/details/com.interversehq.qView 6. https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/#_app_data_validate_usage 7. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_file_install_usage 8. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation -- 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=2068693 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
