This sounds like it should be useful. I'll try and guide you through fixing the current problems, and then I'll sponsor you when the review is complete if everything is satisfactory.

Getting a bit late here, so just some preliminary guidelines:

- example files: these should be part of the documentation, perhaps for the -devel subpackage or -doc, but not in the base package, as they are really meant for developers' use. Sometimes the standard Makefile results in the examples being built as part of the build process; the easiest way out is for you to make a copy somewhere else within the source tree, maybe in %prep, before the 'make' invocation. Then when packaging, pick this copy of examples, rather than the original

- Hidden files: sometimes developers accidentally package editor-created temporary files or (ugh!) OS X metadata files. That's why version-control tools like hg and git have commands to create archive tarballs, but there's nothing us packagers can do apart from cleaning up. You'd want to do this in %install, after make install

- Arch-dependent-file: hmm. this, it's hard to say without knowing the specific case (wouldn't want to give you a half-asleep answer either).

Let me know if there's anything else that's unclear -- I'll give my preliminary review, with more concrete suggestion, tomorrow, but feel free to update the package before then, of course.

Thanks for answering my request.

I didn't know I could get away with not compiling the tests/examples. I patched the configure/Makefiles to not compile them and got rid of all the rpmlint warnings/errors.

Here are the new files:
http://mapleoin.fedorapeople.org/pkgs/telepathy-qt4/telepathy-qt4.spec
http://mapleoin.fedorapeople.org/pkgs/telepathy-qt4/telepathy-qt4-0.1.10-2.fc11.src.rpm

Thanks for volunteering to sponsor me! I thought I had already found a sponsor, but it seems he's too busy now. It would be great if you could sponsor me. I have a few more packages you can look at. More info here: https://bugzilla.redhat.com/show_bug.cgi?id=519282

thanks!

Yes, I have been busy. :-( Classes have just started and I am still playing catchup. :-)

And I would not mind if Michel Salim sponsors you, after all this is not a popularity contest. ;-)

Ionuț please note that in order to be sponsored your reviews should be more verbose instead of simply saying that they follow the guidelines.

There are several examples in other review that I can give as example. For a first time packager it is important to show that you know the issues that are being dealt when building a package.

(In reply to comment #3)
> And I would not mind if Michel Salim sponsors you, after all this is not a
> popularity contest. ;-)
> 
Well, I would not want to intrude. I only stepped in because the review looked to not be assigned to anyone, so if you want to be the official reviewer/sponsor, be my guest :)

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

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.

  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

  any reason tests are not run?

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

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


OK source matches upstream
$ sha1sum telepathy-qt4-0.1.10.tar.gz ../SOURCES/telepathy-qt4-0.1.10.tar.gz 
15f269048f1807bb989c57f84c118b9ac9599a10  telepathy-qt4-0.1.10.tar.gz
15f269048f1807bb989c57f84c118b9ac9599a10  ../SOURCES/telepathy-qt4-0.1.10.tar.gz

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

(In reply to comment #5)
> > 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.
>
I see. Well, if you intend for the users to be able to build the tests, then it should have been a Requires:, not a BuildRequires: . But it's better to get the tests run as part of the build process anyway.

> 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

The patch can be cleaned up further: it appears that only the last part, that modifies Makefile.in to not build examples, need to be kept.

You can drop all the touch scriptlets too.

- the Requires: qt in the main package is unnecessary, as your package currently only generates static library archives. If/when it generates dynamic .so.* files, the requirement on the correct libqt will be automatically computed by RPM anyway.

Also, your -devel package currently Requires: %{name} = %{version}-%{release}, but the main package is not actually generated (because it contains no files). You'd want to remove that requirement. And modify it for -doc, to require the -devel subpackage instead.

(and hold on for a bit on the last paragraph -- I'll clarify on the packaging list, just in case. static libs don't come often).

(In reply to comment #6)
> Also, your -devel package currently Requires: %{name} = %{version}-%{release},
> but the main package is not actually generated (because it contains no files).
> You'd want to remove that requirement. And modify it for -doc, to require the
> -devel subpackage instead.

This bit is actually slightly incorrect (or malformed): the main package isn't generated since it does not have a %files section. However, had you put in an empty %files section, e.g.

 %files

then the main package would be generated, even though it doesn't contain any files.

**

Instead of
 %exclude %{_libdir}/*.la
you can just run run
 rm -f %{_libdir}/*.la
at the end of %install, which IMHO is a cleaner solution. 

**

 %{__chmod} 0644 tests/lib/account-manager.py
should be in %prep, shouldn't it?

**

I think you are missing Requires: telepathy-filesystem in the -devel package for dir ownership.

Could you contact upstream and find out if there is a reason this library currently only builds a static archive? If the only reason is that it's not implemented yet, we could update the build scripts and send the changes back to upstream, and avoid shipping a static package.

Thank you Michel and Jussi,

I followed the packaging list mails and I guess that removing the -doc package and keeping the virtual -static requires on the -devel package is the way to go, at least for now.

I've hopefully cleared all the other issues you mentioned aswell.

> Instead of
>  %exclude %{_libdir}/*.la
> you can just run run
>  rm -f %{_libdir}/*.la
> at the end of %install, which IMHO is a cleaner solution. 

Doing that anywhere in the install section, somehow fails:

RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/lib/libtelepathy-qt4.la

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

* Sat Oct  3 2009 Ionuț Arțăriși <mapleoin> - 0.1.10-4
- Replaced patch with simpler sed command
- Removed -doc subpackage
- Removed qt Requires
- Added telepathy-filesystem Requires


I'm attaching the log of a conversation I had on the
#telepathy.net channel.

After reading a bit more in the README and on the project's website, I suppose
it would be a good idea to put this package to sleep for a few months until
the upstream developers come up with a stable version which will probably also
have dynamic libraries. Right now it seems to me that they don't really want
it distributed.

Created attachment 363552 [details]
#telepathy.log about the static libraries in telepathy-qt4

(In reply to comment #9)
> > Instead of
> >  %exclude %{_libdir}/*.la
> > you can just run run
> >  rm -f %{_libdir}/*.la
> > at the end of %install, which IMHO is a cleaner solution. 
> 
> Doing that anywhere in the install section, somehow fails:
> 
> RPM build errors:
>     Installed (but unpackaged) file(s) found:
>    /usr/lib/libtelepathy-qt4.la

Ugh. I meant, of course
 rm -f %{buildroot}%{_libdir}/*.la
as you won't be (at least SHOULD NOT be) able to remove files from %{_libdir}..

Right. Sorry, I don't know why I didn't see that one.

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

* Sat Oct  3 2009 Ionuț Arțăriși <mapleoin> - 0.1.10-5
- transformed exclude into a rm -f

I'm not interested in maintaining this package any more. So I'm setting it free for anyone to take.