[Bug 520663] Review Request: telepathy-qt4 - Qt4 bindings for telepathy

bugzilla at redhat.com bugzilla at redhat.com
Fri Oct 2 15:57:58 UTC 2009


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=520663





--- Comment #5 from Ionuț Arțăriși <mapleoin at fedoraproject.org>  2009-10-02 11:57:56 EDT ---
Thank you, Michel!

> • rpmlint
> OK package name
> OK spec file name
> OK package guideline-compliant
> OK license complies with guidelines
> FIX license field accurate
>  should be LGPLv2+

Fixed  

> OK license file not deleted
> • spec in US English
> FIX spec legible
> Patch0 should not use %{version}, but hardcode the version number instead.
> Sometimes patches continue to apply unchanged for several versions, and thus
> you don't want to have to keep updating the patch filename.
Fixed  

> regarding configure.ac: why modify both configure.ac and configure? If you
> only modify the latter, you don't have to play tricks with timestamps
Fixed  

>  any reason tests are not run?
Fixed  

> Why does doc require telepathy-farsight-devel to build?

I was including tests in the -doc package so I thought it was fair to keep the
BuildRequires in there. I deleted the tests now and moved the BuildRequires
for them in the main BuildRequires section.

>  Oh, and the doc subpackage should be declared "BuildArch: noarch" too.
Fixed

http://mapleoin.fedorapeople.org/pkgs/telepathy-qt4/telepathy-qt4.spec.2
http://mapleoin.fedorapeople.org/pkgs/telepathy-qt4/telepathy-qt4-0.1.10-3.fc11.src.rpm

* Tue Sep 29 2009 Ionuț Arțăriși <mapleoin at lavabit.com> - 0.1.10-3
- Changed license
- Don't mess with the tests
- Remove hidden directories
- Add more dependencies in order to build tests and examples
- Don't include tests in the final rpm
- Added virtual -static package
- Mark -doc package as BuildArch: noarch

-- 
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.




More information about the Fedora-package-review mailing list