Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 228272 - Review Request: amarokFS - Simple, nice looking full screen front-end for Amarok
Summary: Review Request: amarokFS - Simple, nice looking full screen front-end for Amarok
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Aurelien Bompard
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-02-12 12:49 UTC by Francois Aucamp
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-02-19 22:13:24 UTC
Type: ---
Embargoed:
gauret: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Francois Aucamp 2007-02-12 12:49:48 UTC
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 22:11:34 UTC
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 13:53:32 UTC
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 06:29:00 UTC
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 07:51:54 UTC
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 22:04:46 UTC
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 07:08:38 UTC
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 22:13:24 UTC
Built successfully on -devel; closing ticket and requesting additional branches
via CVS flags:

FC-5 FC-6


Comment 8 Warren Togami 2007-02-19 22:17:20 UTC
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 22:23:49 UTC
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 22:50:54 UTC
oops, OK fixing this.


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