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=542313 Rex Dieter <rdieter@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |rdieter@xxxxxxxxxxxx Flag| |fedora-review?, | |needinfo?(supercyper@xxxxxx | |m) --- Comment #2 from Rex Dieter <rdieter@xxxxxxxxxxxx> 2010-01-20 16:20:20 EST --- A few initial comments: 1. SHOULD : rename pacakge to lower case (if for nothing else, to keep things simple). 2. MUST drop Requires(post,postun): /sbin/ldconfig not needed, rpm will pick that up automatically 3. SHOULD , stuff like this belongs in %prep, not %build, rm -rf example/.svn rm -rf test/.svn sed -i -e "s/INSTALLBASE\/LIB/INSTALLBASE\/%{_lib}/g" src/src.pro 4. SHOULD use available qt4-based rpm macros, like %_qt4_bindir (ie, do export PATH=%{_qt4_bindir}:$PATH ) %_qt4_qmake See /etc/rpm/macros.qt4 for details Otherwise, the rest is fairly simple and looks pretty good. Fix that up, and I can approve this and sponsor you. (while we're at it, what's your FAS username?) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review