Bug 228272 - Review Request: amarokFS - Simple, nice looking full screen front-end for Amarok
Review Request: amarokFS - Simple, nice looking full screen front-end for Amarok
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Aurelien Bompard
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-02-12 07:49 EST by Francois Aucamp
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-19 17:13:24 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
gauret: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Francois Aucamp 2007-02-12 07:49:48 EST
Spec URL: http://www.snoekie.com/rpm/amarokFS.spec
SRPM URL: http://www.snoekie.com/rpm/amarokFS-0.4.2-1.src.rpm
Description:
AmarokFS (Amarok Full Screen) is a full screen front-end for Amarok that
provides a simple and nice looking graphical user interface. It is very
suitable for parties and public terminals. The front-end's appearance can be
customized using themes.

This package also contains a script that allows AmarokFS to be launched
from within Amarok itself.


Package builds in mock (fc6/i386) and on x86_64. rpmlint is silent.
Comment 1 Aurelien Bompard 2007-02-12 17:11:34 EST
Needs work:
* Source URLs are:
http://www.kde-apps.org/CONTENT/content-files/52641-amarokFS-qt3-0.4.2.tar.gz
* An icon is installed directly under %_datadir/icons
(/usr/share/icons/amarokFS.png). Please use the hicolor directory and its
sub-directories, as required by the spec (al least a 48x48 icon)
* Scriptlets: missing "gtk-update-icon-cache" in %post and %postun (wiki:
ScriptletSnippets)

Minor:
* QT environment variable are not sourced
* Desktop file: the Categories tag should not contain Application any more
  (wiki: PackagingGuidelines#desktop)
* The package should contain the text of the license, please ask the author to
include it in the tarball. It's not a blocker for the package, but it would be
better.
Comment 2 Francois Aucamp 2007-02-13 08:53:32 EST
Thanks for the feedback!

New build:
Spec URL: http://www.snoekie.com/rpm/amarokFS.spec
SRPM URL: http://www.snoekie.com/rpm/amarokFS-0.4.2-2.src.rpm

Changes:
- Added BuildRequires: desktop-file-utils
- Fixed source URLs
- Removed "Application" from .desktop file's "Categories" tag
- Install application icon to correct location
- Added KDE/GTK icon cache update scriptlets
- Cleaned up the application's qmake file a bit


(In reply to comment #1)
> Needs work:
> * QT environment variable are not sourced

I am not following you here - the package is being built using qmake; AFAIK the
"CONFIG += qt" line in the qmake project file takes care of this?
I have cleaned up the .pro file accordingly to make it more clear, though. 

> * The package should contain the text of the license, please ask the author to
> include it in the tarball. It's not a blocker for the package, but it would be
> better.

I agree. The developer seems to be away on holiday at the moment, but I will
mail him about this; I also plan on submitting some of the patches/bugfixes I
added to this application once the package is approved.
Comment 3 Aurelien Bompard 2007-02-14 01:29:00 EST
Please also install a 48x48 icon, it is required by the spec. Just use convert
on the 128x128 one (and remember to add ImageMagick as a BuildRequirement).
Otherwise it looks fine, good job.
Comment 4 Francois Aucamp 2007-02-14 02:51:54 EST
Done.

New build:
Spec URL: http://www.snoekie.com/rpm/amarokFS.spec
SRPM URL: http://www.snoekie.com/rpm/amarokFS-0.4.2-3.src.rpm

Changes:
- Added BuildRequires: ImageMagick
- Create and install a 48x48 icon
Comment 5 Aurelien Bompard 2007-02-14 17:04:46 EST
Review for release 3.fc6:
* RPM name is OK
* Source 52641-amarokFS-qt3-0.4.2.tar.gz is the same as upstream
* Source 53125-amarokFS.amarokscript.tar is the same as upstream
* Builds fine in mock
* rpmlints look OK
* File lists look OK
* Works fine

APPROVED 

(35 automatic checks have been run by fedora-qa)
Comment 6 Francois Aucamp 2007-02-15 02:08:38 EST
Thanks for the review!
I will close the review ticket as soon as all branches requested from 
"CVSSyncNeeded" are granted.
Comment 7 Francois Aucamp 2007-02-19 17:13:24 EST
Built successfully on -devel; closing ticket and requesting additional branches
via CVS flags:

FC-5 FC-6
Comment 8 Warren Togami 2007-02-19 17:17:20 EST
I believe your package already has those branches as empty directories.   You
should be able to check it out fresh with those directories, add your files,
checkin, tag and build.  Is this not the case?
Comment 9 Francois Aucamp 2007-02-19 17:23:49 EST
Wow, that was quick ;-)

Nope, sadly all I have is a devel branch:

$ cvs co amarokFS
cvs checkout: Updating amarokFS
U amarokFS/Makefile
U amarokFS/import.log
U amarokFS/pkg.acl
cvs checkout: Updating amarokFS/devel
U amarokFS/devel/.cvsignore
U amarokFS/devel/Makefile
U amarokFS/devel/amarokFS-0.4.2-config_dialog_layout.patch
U amarokFS/devel/amarokFS-0.4.2-fedora_paths.patch
U amarokFS/devel/amarokFS-0.4.2-large_cover_images.patch
U amarokFS/devel/amarokFS-0.4.2-start_amarok.patch
U amarokFS/devel/amarokFS-0.4.2-theme_howto.patch
U amarokFS/devel/amarokFS-0.4.2-theme_prev_button.patch
U amarokFS/devel/amarokFS.spec
U amarokFS/devel/sources
cvs checkout: Updating common
U common/Makefile
U common/Makefile.common
U common/branches
U common/cvs-import.sh
Comment 10 Warren Togami 2007-02-19 17:50:54 EST
oops, OK fixing this.

Note You need to log in before you can comment on or make changes to this bug.