Good:
+ Rpmlint doesn't complaint source rpm.
+ Local build works fine.
+ Tar ball in source rpm matches with upstream
+ Local install/uninstall works fine.
+ Rpmlint of installed rpm doesn't complaints.
+ Programm starts properly.
+ Mock build works fine.

Bad:
- Debuginfo package is empty.
- QT environment variable are not sourced.
- Desktop file: the Categories tag should contain Application
  (wiki: PackagingGuidelines#desktop)
- Missing dependancy on update-desktop-database for %post (package
desktop-file-utils)
- Missing dependancy on update-desktop-database for %postun (package
desktop-file-utils)

(In reply to comment #1)
> Bad:
> - Debuginfo package is empty.

What can I do?
I don't kknow why it is empty.

> - QT environment variable are not sourced.

Why sourcing QT environment variable

> - Desktop file: the Categories tag should contain Application
>   (wiki: PackagingGuidelines#desktop)

I see a sample, not sure  this is a MUST.

> - Missing dependancy on update-desktop-database for %post (package
> desktop-file-utils)
> - Missing dependancy on update-desktop-database for %postun (package
> desktop-file-utils)

This BuildRequires is no more needed since FC5
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef

1.) Don't script the executables. Please use the gcc -g compiler flag.

2.) To be sure to compile with the right QT version.

4.) OK.

(In reply to comment #3)
> 1.) Don't script the executables.

Sorry, I don't understand.

> Please use the gcc -g compiler flag.

Just after %configure, I made:
%{__sed} -i -e 's/CC = gcc/CC = gcc -g/' Makefile
%{__sed} -i -e 's/CPP = gcc -E/CPP = gcc -E -g/' Makefile
%{__sed} -i -e 's/CXX = g++/CXX = g++ -g/' Makefile
%{__sed} -i -e 's/CXXCPP = g++ -E/CXXCPP = g++ -E -g/' Makefile
%{__sed} -i -e 's/CC = gcc/CC = gcc -g/' %{name}/Makefile
%{__sed} -i -e 's/CPP = gcc -E/CPP = gcc -E -g/' %{name}/Makefile
%{__sed} -i -e 's/CXX = g++/CXX = g++ -g/' %{name}/Makefile
%{__sed} -i -e 's/CXXCPP = g++ -E/CXXCPP = g++ -E -g/' %{name}/Makefile

debug-info rpm still empty

> 2.) To be sure to compile with the right QT version.

It seems to me there is only one QT version on each FC?, the last updated.
The only problem I encountred was to buils under FC4 on arch x86_64.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=169132

(In reply to comment #1)
> Bad:
> - Debuginfo package is empty.

I found.
That was because of make install-strip

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/pikloops.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/pikloops-0.2.1-2.src.rpm

%changelog
* Thu Sep 28 2006  Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.2.1-2
  - Don't make install-strip

Usually the files under /etc/profile.d is correctly "source"d
(you can check this by trying to add "set" in the first line of
%build). So "source"ing /etc/profile.d/qt.sh is not necessary.

This can be checked by a example of kbackup:
http://buildsys.fedoraproject.org/logs/fedora-development-extras/18479-kbackup-0.5-6.fc6/x86_64/build.log
(this is reviewed by me).

By the way, you should add %?dist tag.

(In reply to comment #7)
> 
> By the way, you should add %?dist tag.
> 

Oups...

SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/pikloops-0.2.1-3.src.rpm

One question.

Why does /usr/share/applications/kde/pikloops.desktop have
OnlyShowIn=KDE; ?

I tried to remove this, and PiKLoops icon appears on "Programming"
session, so I think this can be shown in GNOME, too.

Other things seem to be okay.

(In reply to comment #9)
> One question.
> 
> Why does /usr/share/applications/kde/pikloops.desktop have
> OnlyShowIn=KDE; ?
> 
> I tried to remove this, and PiKLoops icon appears on "Programming"
> session, so I think this can be shown in GNOME, too.

It seems to me that was to prevent duplicate desktop entry in menu.
Do I have to remove it?

(In reply to comment #10)
> (In reply to comment #9)
> > One question.
> > 
> > Why does /usr/share/applications/kde/pikloops.desktop have
> > OnlyShowIn=KDE; ?
> > 
> It seems to me that was to prevent duplicate desktop entry in menu.
> Do I have to remove it?

Do you see some duplicate in KDE? In GNOME (I use GNOME), when the line
OnlyShowIn=KDE; is removed, the PiKLoops seems to appear only in
"Programming" entry and to have no duplicates. 

(In reply to comment #11)
> 
> Do you see some duplicate in KDE?

No, only one.
I don't understand why I have to relogin to see the new entry in the menu...

> In GNOME (I use GNOME), when the line
> OnlyShowIn=KDE; is removed, the PiKLoops seems to appear only in
> "Programming" entry and to have no duplicates. 

And you see nothing if it isn't removed?

(In reply to comment #12)
> (In reply to comment #11)
> > 
> > Do you see some duplicate in KDE?
> 
> No, only one.
> I don't understand why I have to relogin to see the new entry in the menu...
Umm.. I don't know either because I use GNOME...
 
> > In GNOME (I use GNOME), when the line
> > OnlyShowIn=KDE; is removed, the PiKLoops seems to appear only in
> > "Programming" entry and to have no duplicates. 
> 
> And you see nothing if it isn't removed?

Yes, actually I see nothing.

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > 
> > > Do you see some duplicate in KDE?
> > 
> > No, only one.
> > I don't understand why I have to relogin to see the new entry in the 
menu...
> Umm.. I don't know either because I use GNOME...
>  
> > > In GNOME (I use GNOME), when the line
> > > OnlyShowIn=KDE; is removed, the PiKLoops seems to appear only in
> > > "Programming" entry and to have no duplicates. 
> > 
> > And you see nothing if it isn't removed?
> 
> Yes, actually I see nothing.

OK, I'll update the desktop file.

Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/pikloops.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/pikloops-0.2.1-4.src.rpm

%changelog
* Thu Sep 28 2006  Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.2.1-4
  - Update %%{SOURCE1} which don't contain "OnlyShowIn=KDE;" any more

(In reply to comment #14)
> > > I don't understand why I have to relogin to see the new entry in the 
> menu...
> > Umm.. I don't know either because I use GNOME...

Grrrr! I found... That was because there wasn't "Application" in categories.

But perhaps now, you could have duplicate entry. Let me know.
Spec URL: 
http://linuxelectronique.free.fr/download/fedora/5/SPECS/pikloops.spec
SRPM URL: 
http://linuxelectronique.free.fr/download/fedora/5/SRPMS/pikloops-0.2.1-5.src.rpm

%changelog
* Thu Sep 28 2006  Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.2.1-5
  - Update %%{SOURCE1} to add Application category

(In reply to comment #16)
> (In reply to comment #14)
 
> But perhaps now, you could have duplicate entry. Let me know.

The result is: only one entry is shown, no duplicate.
I will check if the other things to be fixed is left.

Well, accroding to 
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets :
desktop file does not have mimetype key, so
calling update-desktop-database is not necessary.

Other things are okay.
------------------------------------------------
 This package (pikloops) is APPROVED by me.

OK, thanks for the review!

%changelog
* Fri Sep 29 2006  Alain Portal <aportal[AT]univ-montp2[DOT]fr> 0.2.1-6
  - Remove useless update-desktop-database

Package Change Request
======================
Package Name: pikloops
Updated Fedora Owners: alain.portal

Please, add my home email in comps because I'm on vacation for 6 weeks.

Sorry, we can't do this.  Owners must match your Bugzilla account.

What choice do you let me?
 - ask alain.portal AT free.fr for co-maintaining the package?
 - orphaning all packages?

If I can't receive bug reports when I'm on vacation, sure, I can't be a Fedora 
contributor.
I'll never check my work email (my Fedora account) while I'm on vacation.

Package Change Request
=======================
Package Name: pikloops
Short Description: Code generator for PIC delays
Owners: chitlesh
Branches: EL-5 EL-6

cvs done