Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: <main package name here> - <short summary here>
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Summary: Review Request: <main package name here> - <short summary here> Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: zhtx10@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://ekd123.fedorapeople.org/iceplayer.spec SRPM URL: http://ekd123.fedorapeople.org/iceplayer-4.0.4-7.fc14.src.rpm Description: A simple media player. It can download lyrics and show them automatically, and supports themes, ID3, Equalizer and more.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Mike Ma zhtx10@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zhtx10@gmail.com Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #1 from Mike Ma zhtx10@gmail.com 2011-03-16 10:41:26 EDT --- mike@mike-laptop SPECS$ rpmlint iceplayer.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
I tried rpmlint, there is no error or warning.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
William Lima wlima@primate.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |wlima@primate.com.br
--- Comment #2 from William Lima wlima@primate.com.br 2011-03-16 10:43:53 EDT --- Please fix your 'Review Summary'.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Mike Ma zhtx10@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: <main |Review Request: iceplayer - |package name here> - <short |A simple music player for |summary here> |Linux
--- Comment #3 from Mike Ma zhtx10@gmail.com 2011-03-16 10:49:57 EDT --- Everything up-to-date.
New SRPM: http://ekd123.fedorapeople.org/iceplayer-4.0.4-9.fc14.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #4 from Mike Ma zhtx10@gmail.com 2011-03-19 04:48:54 EDT --- Everything up-to-date.
New SRPM: http://ekd123.fedorapeople.org/iceplayer-4.0.4-11.fc14.src.rpm Spec file: http://ekd123.fedorapeople.org/iceplayer.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #5 from Mike Ma zhtx10@gmail.com 2011-03-19 04:49:42 EDT --- (In reply to comment #4)
Everything up-to-date.
New SRPM: http://ekd123.fedorapeople.org/iceplayer-4.0.4-11.fc14.src.rpm Spec file: http://ekd123.fedorapeople.org/iceplayer.spec
that is version 4.0.4-20110318.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #6 from Mike Ma zhtx10@gmail.com 2011-03-26 02:12:10 EDT --- Removed date. Now can build in Koji Build System.
New SRPM: http://ekd123.fedorapeople.org/iceplayer-4.0.4-12.fc14.src.rpm Spec file: http://ekd123.fedorapeople.org/iceplayer.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Fabian Affolter fabian@bernewireless.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fabian@bernewireless.net
--- Comment #7 from Fabian Affolter fabian@bernewireless.net 2011-04-02 10:13:24 EDT --- Just some comments on your spec file:
- The description is too long. Do not exceed 80 characters per line. I think that rpmlint will complain about that anyway. - Check https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files for details about installation of .desktop files. - There are some typos in the .desktop files. And nobody cares where a piece of software is coming from. The .desktop files points to an icon but your are not installing the icon in your spec file. - The update of the desktop database is missing, see https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database - Isn't RPM picking the Requires: automatically? - The "Vendor:" tag is not needed. - The "Release:" tag should contain the release of the spec file. Please check https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Version for details.
I strongly suggest that you read the Fedora Packaging guidelines. https://fedoraproject.org/wiki/Packaging:Guidelines
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #8 from Mike Ma zhtx10@gmail.com 2011-04-02 11:58:21 EDT --- Oh, i'm wrong.
OK. Fixed all of the wrongs (and changed the .desktop file, reported a bug about this to developer). Thank you for all.
... well, i can't understand "Isn't RPM picking the Requires: automatically?", sorry.
It works well for me.
$ rpmlint iceplayer.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
New SRPM: http://ekd123.fedorapeople.org/iceplayer-4.0.4-13.fc14.src.rpm New SPEC: http://ekd123.fedorapeople.org/iceplayer.spec
Please check.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #9 from Fabian Affolter fabian@bernewireless.net 2011-04-02 17:23:08 EDT --- (In reply to comment #8)
OK. Fixed all of the wrongs (and changed the .desktop file, reported a bug about this to developer). Thank you for all.
... well, i can't understand "Isn't RPM picking the Requires: automatically?", sorry.
RPM is able to pick the requirements of a package in most cases automatically. Remove the packages named as "Requires:" and see what happens.
$ rpmlint iceplayer.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint should be run against the packages too.
[fab@laptop023 x86_64]$ rpmlint iceplayer* iceplayer.x86_64: E: explicit-lib-dependency libnotify iceplayer.x86_64: W: spelling-error %description -l en_US linux -> Linux iceplayer.x86_64: E: description-line-too-long C iceplayer is a open-source linux music player. It can play music. And it's a free software. iceplayer.x86_64: W: spurious-executable-perm /usr/share/doc/iceplayer-4.0.4/AUTHORS iceplayer.x86_64: W: spurious-executable-perm /usr/share/doc/iceplayer-4.0.4/COPYING iceplayer.x86_64: W: no-manual-page-for-binary iceplayer iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/desktop_lrc.h iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/keybinding.h iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/download.c iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/list.h iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/equalizer.h 2 packages and 0 specfiles checked; 2 errors, 9 warnings.
- Source0: http://ekd123.fedorapeople.org/%%7Bname%7D-%%7Bversion%7D-20110318.tar.gz is wrong. This must point to the upstream location of the source code. - The naming of the package is wrong. Upstream is releasing different version under the same version number but with a new date. Your package must reflect that circumstance. https://fedoraproject.org/wiki/Packaging:NamingGuidelines - desktop-file-validate is not needed. - mimeinfo is for XML files. You don't need this. - There is a skin directory. Have you consider to put those skins in a separate subpackage? - The build of your package failed on Koji http://koji.fedoraproject.org/koji/taskinfo?taskID=2969153 - The interface of iceplayer is in Chinese (i think). At the moment I'm not sure if this will become an issue.
I'm not a sponsor. I suggest that you follow https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #10 from Mike Ma zhtx10@gmail.com 2011-04-03 05:21:47 EDT --- - The official package is not maked by "make dist", and I added some patches. - 0_o This is latest version. - Yes. It's a Chinese software and will add i18n-support later. fcitx is a Chinese input software, it still can install by yum. (if can't enter repo, I can try put them into FedoraPeople or RPMFusion). - Well, I only keep gstreamer.
OK, deleted some and added a new subpackage, called "iceplayer-data".
mike@mike-laptop SPECS$ rpmlint iceplayer.spec ../SRPMS/iceplayer-4.0.4-14.fc14.src.rpm ../RPMS/x86_64/iceplayer-* iceplayer.x86_64: W: spurious-executable-perm /usr/share/doc/iceplayer-4.0.4/COPYING iceplayer.x86_64: W: spurious-executable-perm /usr/share/doc/iceplayer-4.0.4/AUTHORS iceplayer.x86_64: W: no-manual-page-for-binary iceplayer iceplayer-data.x86_64: W: no-documentation iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/desktop_lrc.h iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/keybinding.h iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/download.c iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/list.h iceplayer-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/iceplayer-4.0.4-20110318/equalizer.h 4 packages and 1 specfiles checked; 0 errors, 9 warnings. (I don't know why those happend)
Koji build page: http://koji.fedoraproject.org/koji/taskinfo?taskID=2969801 2969801 build (dist-f14, iceplayer-4.0.4-14.fc14.src.rpm) completed successfully
New SPEC: http://ekd123.fedorapeople.org/iceplayer.spec New SRPM: http://ekd123.fedorapeople.org/iceplayer-4.0.4-14.fc14.src.rpm
It works well
(Sorry for my poor English)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #11 from Mike Ma zhtx10@gmail.com 2011-04-05 03:52:06 EDT --- (In reply to comment #9)
- The build of your package failed on Koji
OK. I find the reason. and upload a patch to developers. Now can build in fc15/16.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #12 from Michael Schwendt mschwendt@gmail.com 2011-05-03 04:24:02 EDT --- Just a quick look at the spec file:
So, if the base "iceplayer" package requires "iceplayer-data" and "iceplayer-data" requires" iceplayer", you've created a circular dependency within the packages built from the same src.rpm. Since iceplayer-data is not noarch, why put the data into a separate package instead of the base package?
Requires: %{name}-data gstreamer
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
In particular, the dependency on GStreamer libraries should be automatic already. However, is a dependency on a minimum set of GStreamer Plug-in packages missing?
%files data %defattr (-,root,root,-) %{_datadir}/%{name}/*
https://fedoraproject.org/wiki/Packaging:UnownedDirectories
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #13 from Mike Ma zhtx10@gmail.com 2011-06-07 02:35:22 EDT --- Hi Michael, Sorry for your waiting and thank you for your suggestions.
Could you please take a look at this new spec file? http://ekd123.fedorapeople.org/iceplayer.spec
Thank you very much.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Elder Marco eldermarco@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |eldermarco@gmail.com
--- Comment #14 from Elder Marco eldermarco@gmail.com 2011-07-01 19:34:54 EDT --- Hi,
Just a few comments.
- It's a good idea to add each dependencies on a single line. It's more readable. In this case:
BuildRequires: gtk2-devel BuildRequires: gstreamer-devel BuildRequires: libnotify-devel BuildRequires: desktop-file-utils
- The strings in source code is in Japanese/Chinese .. I don't know, but this is a problem.
- In Source0 field, you could use: Source0: http://iceplayer.googlecode.com/files/%%7Bname%7D-src-%%7Bversion%7D-%%7Bdat...
- Use %{?_smp_mflags} instead of %{_smp_mflags}.
- In %files section, add the files NEWS THANKS in %doc. - For the files, you could use: %{_bindir}/%{name} %{_datadir}/applications/%{name}.desktop %dir %{_datadir}/%{name}/ %{_datadir}/%{name}/*
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
--- Comment #15 from Mike Ma zhtx10@gmail.com 2011-07-02 02:32:21 EDT --- Hi, Elder.
Thanks for your comments.
My computer can't connect to Internet now, so I can't edit the spec file. (sorry. but I'll modify it later.)
Well, this software is in Chinese, and doesn't have i18n now. If that is a problem, can I close the Review Request? (I think FedoraPeople is enough.)
Thank you.
PS, NEWS and THANKS are empty files.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Matthias Runge mrunge@matthias-runge.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mrunge@matthias-runge.de
--- Comment #16 from Matthias Runge mrunge@matthias-runge.de 2012-03-02 15:42:36 EST --- Mike, are you still interested?
The spec file isn't accessible any more.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=688183
Matthias Runge mrunge@matthias-runge.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status Whiteboard| |Stalled Submitter
--- Comment #17 from Matthias Runge mrunge@matthias-runge.de 2012-03-24 15:10:51 EDT --- ping?
package-review@lists.fedoraproject.org