Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/mellowplayer.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/mellowplayer-3.1.0-1.fc27.src.rpm Description: MellowPlayer is a free, open source and cross-platform desktop application that integrates online music services with your desktop. Fedora Account System Username: martinkg
Not a formal review but a couple of points after reading the SPEC: - Don't add: %dir %{_datadir}/icons/hicolor/scalable %dir %{_datadir}/icons/hicolor/scalable/apps These directories should be owned by the Requires to hicolor-icon-theme - Please note last week change in the guidelines regarding Appdata. It was announced on the devel-announce mailing list: Appstream metadata guidelines were updated to reflect the new location into which appdata files should be placed. * https://fedoraproject.org/wiki/Packaging:AppData * https://pagure.io/packaging-committee/issue/704 Per the new guidelines, appdata files must now be installed in %{_datadir}/metainfo/ instead of %{_datadir}/appdata/ - You must run the Icon cache scriplet: https://fedoraproject.org/wiki/Packaging:Scriptlets?rd=Packaging:ScriptletSnippets#Icon_Cache %post /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : %postun if [ $1 -eq 0 ] ; then /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : fi %posttrans /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : - Add the changelog and authors to %doc: %doc AUTHORS.md CHANGELOG.md README.md - Use this simplified URL: Source0: https://github.com/ColinDuquesnoy/%{name}/archive/%{version}/%{name}-%{version}.tar.gz - Regarding: # Build is broken on: ExcludeArch: ppc64le ppc64 s390x Build is not "broken", MellowPlayer is using Qt Web Engine, which is only available on some arches. However this is not how you should handle it, instead we have a specific macro: ExclusiveArch: %{qt5_qtwebengine_arches}
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/mellowplayer.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/mellowplayer-3.1.0-2.fc27.src.rpm %changelog * Sun Nov 05 2017 Martin Gansser <martinkg> - 3.1.0-2 - Don't add: %%dir %%{_datadir}/icons/hicolor/scalable and %%dir %%{_datadir}/icons/hicolor/scalable/apps These directories should be owned by the Requires to hicolor-icon-theme - Per the new guidelines, appdata files must now be installed in %%{_datadir}/metainfo/ instead of %%{_datadir}/appdata/ - Add Icon cache scriplet - Add changelog and authors to %%doc - Use simplified URL - Use ExclusiveArch: %%{qt5_qtwebengine_arches} due Qt Web Engine is only available on some arches
- Package uses either %{buildroot} or $RPM_BUILD_ROOT Note: Using both %{buildroot} and $RPM_BUILD_ROOT See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros - Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 17756160 bytes in 107 files. See: http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/mellowplayer.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/mellowplayer-3.1.0-3.fc27.src.rpm %changelog * Sun Nov 05 2017 Martin Gansser <martinkg> - 3.1.0-3 - Use %%{buildroot} macro for consistency - Large documentation must go in a -doc subpackage
You should keep %{_mandir}/man1/%{rname}.1.* along with the binary in the main package. Package accepted otherwise..
(In reply to Robert-André Mauchin from comment #5) > You should keep %{_mandir}/man1/%{rname}.1.* along with the binary in the > main package. done, no extra package upload. > > Package accepted otherwise.. thanks for the review
(fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/mellowplayer