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 <rdie...@math.unl.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nob...@fedoraproject.org    |rdie...@math.unl.edu
               Flag|                            |fedora-review?,
                   |                            |needinfo?(supercy...@163.co
                   |                            |m)

--- Comment #2 from Rex Dieter <rdie...@math.unl.edu> 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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to