[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version||sayonara-0.6.2-4.svn1021.fc ||21 Resolution|NEXTRELEASE |ERRATA --- Comment #17 from Fedora Update System upda...@fedoraproject.org --- sayonara-0.6.2-4.svn1021.fc21 has been pushed to the Fedora 21 stable repository. -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #14 from Jon Ciesla limburg...@gmail.com --- Git done (by process-git-requests). -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 Jon Ciesla limburg...@gmail.com changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 MartinKG mgans...@alice.de changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution|--- |NEXTRELEASE Last Closed||2015-02-17 10:37:26 --- Comment #15 from MartinKG mgans...@alice.de --- package has been built successfully on f21, fc22 and rawhide. -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #16 from Fedora Update System upda...@fedoraproject.org --- sayonara-0.6.2-4.svn1021.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/sayonara-0.6.2-4.svn1021.fc21 -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #10 from Mario Blättermann mario.blaetterm...@gmail.com --- One more issue, rather cosmetic: Buildrequires: curl-devel has to be BuildRequires: curl-devel -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415
--- Comment #9 from Mario Blättermann mario.blaetterm...@gmail.com ---
(In reply to MartinKG from comment #8)
Spec URL: https://www.dropbox.com/s/2cnhsow8i72mput/sayonara.spec?dl=0
SRPM URL:
https://www.dropbox.com/s/g5j7pwwk5omq55c/sayonara-0.6.2-2.svn1016.fc21.src.
rpm?dl=0
Please don't use Dropbox for the file links. It is impossible to use
fedora-review because the links point to the website, not the file itself. It
is no problem for me, I don't use fedora-review at all, but it could be
annoying for other reviewers of your packages. Upload the files to your Fedora
webspace instead.
However, here's the scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=8954463
$ rpmlint -i -v *
sayonara.i686: I: checking
sayonara.i686: I: checking-url http://code.google.com/p/sayonara-player/
(timeout 10 seconds)
sayonara.i686: W: no-manual-page-for-binary sayonara
Each executable in standard binary directories should have a man page.
sayonara.i686: E: invalid-appdata-file /usr/share/appdata/sayonara.appdata.xml
appdata file is not valid, check with appdata-validate
sayonara.src: I: checking
sayonara.src: I: checking-url http://code.google.com/p/sayonara-player/
(timeout 10 seconds)
sayonara.src: W: invalid-url Source0: sayonara-player-r1016.tar.gz
The value should be a valid, public HTTP, HTTPS, or FTP URL.
sayonara.x86_64: I: checking
sayonara.x86_64: I: checking-url http://code.google.com/p/sayonara-player/
(timeout 10 seconds)
sayonara.x86_64: W: no-manual-page-for-binary sayonara
Each executable in standard binary directories should have a man page.
sayonara.x86_64: E: invalid-appdata-file
/usr/share/appdata/sayonara.appdata.xml
appdata file is not valid, check with appdata-validate
sayonara-debuginfo.i686: I: checking
sayonara-debuginfo.i686: I: checking-url
http://code.google.com/p/sayonara-player/ (timeout 10 seconds)
sayonara-debuginfo.x86_64: I: checking
sayonara-debuginfo.x86_64: I: checking-url
http://code.google.com/p/sayonara-player/ (timeout 10 seconds)
sayonara.spec: I: checking
sayonara.spec: W: invalid-url Source0: sayonara-player-r1016.tar.gz
The value should be a valid, public HTTP, HTTPS, or FTP URL.
5 packages and 1 specfiles checked; 2 errors, 4 warnings.
Nothing worth to discuss. Missing man page is not up to you to add, the tarball
is created from SVN, and the appdata file *is* valid, albeit not fully
canonical. OK, here we go:
-
key:
[+] OK
[.] OK, not applicable
[X] needs work
-
[+] MUST: rpmlint must be run on the source rpm and all binary rpms the build
produces. The output should be posted in the review.
[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption.
[X] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual
license.
GPLv3+
[+] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package must be included in %license.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL. Reviewers should use sha256sum for this task as it
is used by the sources file once imported into git. If no upstream URL can be
specified for this package, please see the Source URL Guidelines for how to
deal with this.
$ sha256sum *
Checksums differ, but this happens often in case of a tarball created from
a VCS. I trust you :)
[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in
bugzilla, describing the reason that the package does not compile/build/work on
that architecture. The bug number MUST be placed in a comment, next to the
corresponding ExcludeArch line.
[+] MUST: All build dependencies must be listed in BuildRequires, except for
any that are listed in the exceptions section of the Packaging Guidelines ;
inclusion of those as BuildRequires is optional. Apply common sense.
[+] MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
[.] MUST: Every binary RPM package (or subpackage) which stores shared library
files (not just symlinks) in any of the dynamic linker's default paths,
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 Mario Blättermann mario.blaetterm...@gmail.com changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #12 from Mario Blättermann mario.blaetterm...@gmail.com --- OK, looks fine now. PACKAGE APPROVED! -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 MartinKG mgans...@alice.de changed: What|Removed |Added Flags||fedora-cvs? --- Comment #13 from MartinKG mgans...@alice.de --- @Mario Thanks for the review. New Package SCM Request === Package Name: sayonara Short Description: A lightweight Qt Audio player Owners: martinkg Branches: f21 f22 InitialCC: -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415
--- Comment #11 from MartinKG mgans...@alice.de ---
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/sayonara.spec
SRPM URL:
https://martinkg.fedorapeople.org/Review/SRPMS/sayonara-0.6.2-3.svn1018.fc21.src.rpm
Mon Feb 16 2015 Martin Gansser marti...@fedoraproject.org - 0.6.2-3.svn1018
- rebuild for new svn release
- cosmetic changes
- take ownership of unowned directory %%{_datadir}/%%{name}/translations
- take ownership of unowned directory %%{_datadir}/%%{name}/translations/icons
--
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
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #8 from MartinKG mgans...@alice.de --- (In reply to Mario Blättermann from comment #6) BuildRequires: gstreamer1-devel is redundant, it is pulled in by gstreamer1-plugins-base-devel. Is svn really a runtime requirement? If this points to the tarball creation, it is not needed. deleted According to the licensecheck output, the license is GPLv3+. corrected The package installs icons in /usr/share/icons/hicolor, that's why Requires: hicolor-icon-theme is needed. added The file license.txt needs to be added to %files under %license. added What about the Java stuff in sayonara_client? deleted Would be nice to have an *appdata.xml file to let the program also appear in future gnome-software versions. Maybe you could contact upstream to add one. added, reported upstream From the *.desktop file: Categories=Audio;Music;Player;AudioVideo; The last category is not a secondary one, referring to the Freedesktop specification (see http://standards.freedesktop.org/menu-spec/latest/apas02.html), and actually Sayonara don't seem to be a video player. Better remove this category, or put it as a primary category and remove Audio. corrected new rpm files upload Spec URL: https://www.dropbox.com/s/2cnhsow8i72mput/sayonara.spec?dl=0 SRPM URL: https://www.dropbox.com/s/g5j7pwwk5omq55c/sayonara-0.6.2-2.svn1016.fc21.src.rpm?dl=0 %changelog * Mon Feb 16 2015 Martin Gansser marti...@fedoraproject.org - 0.6.2-2.svn1016 - deleted BR gstreamer1-devel because its redundant - deleted RR svn isn't needed - corrected license tag to GPLv3+ - added RR hicolor-icon-theme - mark license files as %%license where available - added appdata.xml file - modified desktop file Categories - removed java stuff -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 Mario Blättermann mario.blaetterm...@gmail.com changed: What|Removed |Added CC||mario.blaetterm...@gmail.co ||m --- Comment #6 from Mario Blättermann mario.blaetterm...@gmail.com --- BuildRequires: gstreamer1-devel is redundant, it is pulled in by gstreamer1-plugins-base-devel. Is svn really a runtime requirement? If this points to the tarball creation, it is not needed. According to the licensecheck output, the license is GPLv3+. The package installs icons in /usr/share/icons/hicolor, that's why Requires: hicolor-icon-theme is needed. The file license.txt needs to be added to %files under %license. What about the Java stuff in sayonara_client? Would be nice to have an *appdata.xml file to let the program also appear in future gnome-software versions. Maybe you could contact upstream to add one. From the *.desktop file: Categories=Audio;Music;Player;AudioVideo; The last category is not a secondary one, referring to the Freedesktop specification (see http://standards.freedesktop.org/menu-spec/latest/apas02.html), and actually Sayonara don't seem to be a video player. Better remove this category, or put it as a primary category and remove Audio. -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 Mario Blättermann mario.blaetterm...@gmail.com changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|nob...@fedoraproject.org|mario.blaetterm...@gmail.co ||m Alias||sayonara Flags||fedora-review? --- Comment #7 from Mario Blättermann mario.blaetterm...@gmail.com --- Taking this for a full review. -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 Mario Blättermann mario.blaetterm...@gmail.com changed: What|Removed |Added Blocks||928937 (qt-reviews) Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=928937 [Bug 928937] Qt-related package review tracker -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #5 from MartinKG mgans...@alice.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/sayonara.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/sayonara-0.6.2-1.svn1016.fc21.src.rpm %changelog * Fri Feb 13 2015 Martin Gansser marti...@fedoraproject.org - 0.6.2-1.svn1016 - rebuild for new svn release -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #4 from MartinKG mgans...@alice.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/sayonara.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/sayonara-0.4.1-1.4.svn878.fc20.src.rpm %changelog * Mon Sep 01 2014 Martin Gansser marti...@fedoraproject.org - 0.4.1-1.4.svn878 - enabled debugging informations - rebuild for new svn release - set correct file permisson rpmlint sayonara-0.4.1-1.4.svn878.fc20.x86_64.rpm sayonara.x86_64: W: no-manual-page-for-binary sayonara 1 packages and 0 specfiles checked; 0 errors, 1 warnings. rpmlint sayonara-debuginfo-0.4.1-1.4.svn878.fc20.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415
--- Comment #3 from MartinKG mgans...@alice.de ---
(In reply to NIWA Hideyuki from comment #2)
Hi
This is my informal review. I comment about the spec file etc.
1. Source0:sayonara-player-%{svnrev}.tar.gz
URL is necessary for the Source0 tag.
There is no URL available for the recent snv tarball.
2. #
https://sayonara-player.googlecode.com/issues/
attachment?aid=92000name=sayonara-desktop.
patchtoken=ABZ6GAezT8Y7uTPdlcAPejNluH3R5pcUiQ%3A1402298727539
Patch0: sayonara-desktop.patch
#
https://sayonara-player.googlecode.com/issues/
attachment?aid=92001name=sayonara-libdir.
patchtoken=ABZ6GAftNAWKGIQtU8vT9AwhapN1BEXTdw%3A1402298727539
Patch1: sayonara-libdir.patch
When the brief comment is put on the upper part of Patch0 and Patch1
above upstream bug tracker link, it is helpful.
done
3. BuildRequires: cmake
BuildRequires: desktop-file-utils
BuildRequires: qt-devel
BuildRequires: alsa-lib-devel
BuildRequires: gstreamer-devel
BuildRequires: gstreamer-plugins-base-devel
BuildRequires: libnotify-devel
Buildrequires: curl-devel
BuildRequires: libxml2-devel
BuildRequires: taglib-devel
BuildRequires: glib2-devel
Requires: svn
Please add comments on explicit dependencies.
remove unnecessary dependencies
4. rm -rf %{buildroot}/%{_datadir}/menu
Is this processing necessary? Please delete it if it is unnecessary.
its necessary
Spec URL: https://www.dropbox.com/s/2cnhsow8i72mput/sayonara.spec?dl=0
SRPM URL:
https://www.dropbox.com/s/f8vgj2c7whhv4ze/sayonara-0.4.1-1.3.svn870.fc20.src.rpm?dl=0
%changelog
* Fri Aug 29 2014 Martin Gansser marti...@fedoraproject.org -
0.4.1-1.3.svn870
- rebuild for new svn release
- added more comments
rpmlint sayonara-debuginfo-0.4.1-1.3.svn870.fc20.x86_64.rpm
sayonara-debuginfo.x86_64: E: debuginfo-without-sources
1 packages and 0 specfiles checked; 1 errors, 0 warnings.
--
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
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415
NIWA Hideyuki niwa.hidey...@jp.fujitsu.com changed:
What|Removed |Added
CC||niwa.hidey...@jp.fujitsu.co
||m
--- Comment #2 from NIWA Hideyuki niwa.hidey...@jp.fujitsu.com ---
Hi
This is my informal review. I comment about the spec file etc.
1. Source0:sayonara-player-%{svnrev}.tar.gz
URL is necessary for the Source0 tag.
2. #
https://sayonara-player.googlecode.com/issues/attachment?aid=92000name=sayonara-desktop.patchtoken=ABZ6GAezT8Y7uTPdlcAPejNluH3R5pcUiQ%3A1402298727539
Patch0: sayonara-desktop.patch
#
https://sayonara-player.googlecode.com/issues/attachment?aid=92001name=sayonara-libdir.patchtoken=ABZ6GAftNAWKGIQtU8vT9AwhapN1BEXTdw%3A1402298727539
Patch1: sayonara-libdir.patch
When the brief comment is put on the upper part of Patch0 and Patch1
above upstream bug tracker link, it is helpful.
3. BuildRequires: cmake
BuildRequires: desktop-file-utils
BuildRequires: qt-devel
BuildRequires: alsa-lib-devel
BuildRequires: gstreamer-devel
BuildRequires: gstreamer-plugins-base-devel
BuildRequires: libnotify-devel
Buildrequires: curl-devel
BuildRequires: libxml2-devel
BuildRequires: taglib-devel
BuildRequires: glib2-devel
Requires: svn
Please add comments on explicit dependencies.
4. rm -rf %{buildroot}/%{_datadir}/menu
Is this processing necessary? Please delete it if it is unnecessary.
--
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
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1106415] Review Request: sayonara - A lightweight Qt Audio player
https://bugzilla.redhat.com/show_bug.cgi?id=1106415 --- Comment #1 from MartinKG mgans...@alice.de --- Spec URL: http://martinkg.fedorapeople.org/Review/SPECS/sayonara.spec SRPM URL: http://martinkg.fedorapeople.org/Review/SRPMS/sayonara-0.4.0-1.2.svn851.fc20.src.rpm %changelog * Tue Jun 10 2014 Martin Gansser marti...@fedoraproject.org - 0.4.0-1.2.svn851 - rebuild for new svn release - added svn Requirement - corrected svn path -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review
