I'll review it.

Ver. 0.2_beta2 is out. Please, consider updating your srpm, and I'll continue.

http://www.qutim.org/uploads/src/qutim-0.2_beta2.tar.gz

Off course! Many time ago I was try build it, but it is impossible due bug: http://trac.qutim.org/task/258

I directly speak with maintainer, and problem occurred in directly patching bundled copy of gloox library. This way is unacceptable for Fedora. So, waiting until bug resolved.

Ok, understood. Until then, I'm adding NotReady keyword to the whiteboard.

There is build for 0.2 beta svn snapshot in this repo: ftp://volkoff.ru/repo/fedora11/
Here is discussion: http://linuxforum.ru/index.php?showtopic=99429
But perhaps they build it with bundled gloox. Is that bug still unfixed?

As you can see, bug is reopened and still unfixed. If you want speedup - comment these bug in its bugtracker to fix error.
Previous maintainer of Qutim-Jabber plugin is unaccessible for the long time for me. I think it is main reason...

Also, as you known Fedora do not permit bundled libraries and built with it is prohibited.

Ura! We did it!

http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim.spec
http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2-0.15.beta2SVN20091018.fc11.src.rpm

Koji scratchbuild for F-11 failed due to easy-to-fix issue in the %files section:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1776709

Oh, sorry. On my system this macros defined.

http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2-0.16.beta2SVN20091018.fc11.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1778748

BTW, qutim 0.2 is out! 

http://www.qutim.org/forum/viewtopic.php?f=40&t=1335

Could you, please, update your package to the latest version?

Off course! Gloox release 1.0 also was released yesterday. We speak about it with Ruslan and I prepare both updates (I think do that in 1-3 days). But it is not contain major changes except bugfixes and api stability, can we finally start review?? Update process in the future should be persistent continuous process...

Builds fine now.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1791960

REVIEW:

+ rpmlint is not silent:

[petro@Workplace ~]$ rpmlint Desktop/qutim-*
qutim-debuginfo.i586: W: spurious-executable-perm /usr/src/debug/qutim/plugins/icq/clientidentify.h
qutim-debuginfo.i586: W: spurious-executable-perm /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.cpp
qutim-debuginfo.i586: E: wrong-script-end-of-line-encoding /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.cpp
qutim-debuginfo.i586: W: spurious-executable-perm /usr/src/debug/qutim/plugins/icq/buddycaps.h
qutim-debuginfo.i586: W: spurious-executable-perm /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.h
qutim-debuginfo.i586: E: wrong-script-end-of-line-encoding /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.h
qutim-debuginfo.i586: W: spurious-executable-perm /usr/src/debug/qutim/plugins/icq/clientidentify.cpp
qutim-devel.i586: W: no-documentation
qutim-icq.i586: W: no-documentation
qutim-irc.i586: W: no-documentation
qutim-jabber.i586: W: no-documentation
qutim-mrim.i586: W: no-documentation
7 packages and 0 specfiles checked; 2 errors, 10 warnings.
[petro@Workplace ~]$ 

These issues should be fixed at %prep stage.

+ The package is named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

- The package does not fully  meet the Packaging Guidelines. There are some unlisted Requires and some unneeded calls for ldconfig. Also, and most important, I can;t find a package, who provides /usr/share/icons/mini.

So, you should add the following explicit requires:

hicolor-icon-theme (for /usr/share/icons/hicolor/64x64/apps )
cmake (for /usr/share/cmake/Modules )

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines .
+ The License field in the package spec file matches the actual license.
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.

+/- The sources used to build the package must match the upstream source, as provided in the spec URL. Since it's a pre-release, and new version was recently released, I'll wait until you update srpm to ver. 0.2.

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji log above.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files (not just symlinks) in any of the dynamic linker's default paths.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.

- The package must own all directories that it creates. Please, add %{_libdir}/%{name} to the main package's %files section as %dir. Also add %{_includedir}/%{name} as %dir in devel sub-package.

- A Fedora package must not list a file more than once in the spec file's %files listings. Unfortunately, you're listing %{_bindir}/%{name} twice in main package's %files section.

- Permissions on files must be set properly, but there are some rpmlint complaints regarding wrong permissions. See rpmlint log posted above.

+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
0 Anything, the package includes as %doc, does not affect the runtime of the application.
+ Header files are placed in a -devel package.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package does not contain library files with a suffix.
+ The devel package requires the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
+ The packages does NOT contain any .la libtool archives.
+ The package includes a %{name}.desktop file, and that file is properly installed with desktop-file-install in the %install section.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in the package are valid UTF-8.

So, please, fix issues noted above, and I'll continue with review.

Peter, you was right - in release many changes...

(In reply to comment #12)
> Builds fine now.
> 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1791960
> 
> REVIEW:
> 
> + rpmlint is not silent:
> 
> [petro@Workplace ~]$ rpmlint Desktop/qutim-*
> qutim-debuginfo.i586: W: spurious-executable-perm
> /usr/src/debug/qutim/plugins/icq/clientidentify.h
> qutim-debuginfo.i586: W: spurious-executable-perm
> /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.cpp
> qutim-debuginfo.i586: E: wrong-script-end-of-line-encoding
> /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.cpp
> qutim-debuginfo.i586: W: spurious-executable-perm
> /usr/src/debug/qutim/plugins/icq/buddycaps.h
> qutim-debuginfo.i586: W: spurious-executable-perm
> /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.h
> qutim-debuginfo.i586: E: wrong-script-end-of-line-encoding
> /usr/src/debug/qutim/src/3rdparty/qtwin/qtwin.h
> qutim-debuginfo.i586: W: spurious-executable-perm
> /usr/src/debug/qutim/plugins/icq/clientidentify.cpp
> qutim-devel.i586: W: no-documentation
> qutim-icq.i586: W: no-documentation
> qutim-irc.i586: W: no-documentation
> qutim-jabber.i586: W: no-documentation
> qutim-mrim.i586: W: no-documentation
> 7 packages and 0 specfiles checked; 2 errors, 10 warnings.
spurious-executable-perm fixed in %prep.

Add BR dos2unix and fix lineendings qtwin to avoid rpmlint complain wrong-script-end-of-line-encoding. But is it really needed for files inytended for Windows (just delete break build, but it nothing doing on Linux)?

> and some unneeded calls for ldconfig.
Why calls to ldconfig is unneeded?

http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries says:
Whenever possible (and feasible), Fedora Packages containing libraries should build them as shared libraries. In addition, every binary RPM package which contains shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. If the package has multiple subpackages with libraries, each subpackage should also have a %post/%postun section that calls /sbin/ldconfig.

> Also, and most
> important, I can;t find a package, who provides /usr/share/icons/mini.
Hm, yes, I too. May be I simple should delete this file?

> So, you should add the following explicit requires:
> 
> hicolor-icon-theme (for /usr/share/icons/hicolor/64x64/apps )
Added.
> cmake (for /usr/share/cmake/Modules )
Added to -devel subpackage.

> + The License field in the package spec file matches the actual license.
There I find some issues: Several files like (3rdparty/qtsolutions/*, plugins/plugman/libs/QZipReader) is 3dparty and have licensing similar to QT: Commercial/LGPL with exceptions in LGPL_EXCEPTION.txt/GPLv3.
This is free software, but it is not under GPLv2+ license as full package. And I in frustration - can we ship it? If you can't answer on this question, please set "legal" flag for Spot.

> +/- The sources used to build the package must match the upstream source, as
> provided in the spec URL. Since it's a pre-release, and new version was
> recently released, I'll wait until you update srpm to ver. 0.2.
I update it. Also several new plugins appeared in release.

> - A Fedora package must not list a file more than once in the spec file's
> %files listings. Unfortunately, you're listing %{_bindir}/%{name} twice in main
> package's %files section.
Yes, it with comment from old times, when symlink was also been installed.
Fixed.

http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-1.17.fc11.src.rpm

Add epoch in gloox dependency:
http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-1.18.fc11.src.rpm

Delete invoke ldconfig:
http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-1.19.fc11.src.rpm

Still not addressed:

* %global _miconsdir      %{_datadir}/icons/mini
There is no such package in Fedora repositories, which owns this directory. Also your package doesn't own it. This should be fixed. I believe, that this icon should be placed in some other directory than %{_datadir}/icons/mini .

* The macro %{_iconsdir} is superfluous.

%global _iconsdir               %{_datadir}/icons
%global _hiconsdir      %{_datadir}/icons/hicolor

Take a look at both places, where you're using %{_iconsdir} - actually, you should use %{_hiconsdir} macro there. I advice you to shorten the number of macro being used.

* Directory ownership issues:

%{_libdir}/%{name} should be owned by the main package.
%{_includedir}/%{name} should be owned by *-devel sub-package

* Also consider removing entirely %doc sections from plugin sub-packages.

Thank you. All mentioned issue fixed, except one:
> * Also consider removing entirely %doc sections from plugin sub-packages.  
paths fixed instead, and in plugins packages included its docs (only mrim do not have it because it is absent in upstream tarball)

http://koji.fedoraproject.org/koji/taskinfo?taskID=1832695

http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-1.20.fc11.src.rpm

(In reply to comment #17)
> Thank you. All mentioned issue fixed, except one:
> > * Also consider removing entirely %doc sections from plugin sub-packages.  
> paths fixed instead, and in plugins packages included its docs (only mrim do
> not have it because it is absent in upstream tarball)
> 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1832695
> 
> http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-1.20.fc11.src.rpm  

Some new issues :)

* In main package you forgot to mark owned directory %{_libdir}/%{name} as %dir
* I believe, that instead of installing small icon to %{buildroot}/%{_datadir}/icons/hicolor/16x16/ you intended to install to %{buildroot}/%{_datadir}/icons/hicolor/16x16/apps

(In reply to comment #18)
> * In main package you forgot to mark owned directory %{_libdir}/%{name} as %dir
Oh, yes, thank you.
> * I believe, that instead of installing small icon to
> %{buildroot}/%{_datadir}/icons/hicolor/16x16/ you intended to install to
> %{buildroot}/%{_datadir}/icons/hicolor/16x16/apps  
Fixed.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1836516
http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-1.21.fc11.src.rpm

ping?

a small comments regarding package description/summary, imho, it makes little sense to mention "multiplatform" or "on Qt4", (fedora) users won't care (or need to know) about what platforms are supported or the toolkit used.

Cleaning up NotReady, since it is almost ready.

(In reply to comment #21)
> a small comments regarding package description/summary, imho, it makes little
> sense to mention "multiplatform" or "on Qt4", (fedora) users won't care (or
> need to know) about what platforms are supported or the toolkit used.  

I would like to keep at least mention about toolkit, since some users prefer GTK apps over Qt or vice versa.

OK, but please leave it out of the package summary at least, this could be something mentionable in the package description.  that's a lesser of 2 evils.

Ok, I change "multiplatform" on "Multiprotocol" what is more important for IM. Mention of QT4 have worth on my mind too. Summary rewritten.

Release enumeration changed, some comments stripped.

http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim-0.2.0-2.fc11.src.rpm
http://hubbitus.net.ru/rpm/Fedora11/qutim/qutim.spec

Ok, I don't see any outstanding issues so far, so this package is

APPROVED.

Peter, thank you very much for the review.

New Package CVS Request
=======================
Package Name: qutim
Short Description: Multiprotocol (ICQ, Jabber, IRC etc) instant messenger with modern Qt4 interface
Owners: hubbitus
Branches: F-10 F-11 F-12
InitialCC:

We no longer do F-10 branches as it's now end of life. 

otherwise, cvs done.

Thank you, Kevin.

qutim-0.2.0-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/qutim-0.2.0-2.fc12

qutim-0.2.0-2.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update qutim'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2009-13435

Plugins don't work on x86-64. 
Please update plugin directory for x86-64 to "/usr/lib64/qutim/"

(In reply to comment #32)
> Plugins don't work on x86-64. 
> Please update plugin directory for x86-64 to "/usr/lib64/qutim/"  

Aleksey, I'm not sure that I can confirm your report - I just checked qutim-0.2.0-2.fc12.x86_64.rpm, qutim-icq-0.2.0-2.fc12.x86_64.rpm and qutim-jabber-0.2.0-2.fc12.x86_64.rpm - they all contain /usr/lib64/qutim directory (even with proper library in case of plugin sub-package).

Sorry, I wasn't clear enough.

I mean search path for plugins.

My installation gives me the following when run from command line:
Debug: ("/usr/bin/plugins", "/usr/lib/qutim", "/usr/lib/qutim/plugins", "/usr/PlugIns", "/home/nld/.config/qutim/plugins") 
Debug: ("", "../share/qutim", "/home/nld/.config/qutim") 
Debug: "English" "UnitedStates" 
Debug: ("en_US", "en_us", "en") 
Debug: "English" "UnitedStates" 
Debug: ("en_US", "en_us", "en") 
Debug: "en" 
Debug: "QTreeView {  }" 
Warning: QFSFileEngine::open: No file name specified

The first path should contain also "/usr/lib64/qutim/".

Thanks.

Aleksey thank you for the bugreport, but qutim already build and imported and it is not subject of Review Request. Please, for each problems in qutim fill separate bugs. Meantime I'll look on it. You say what plugins does not loaded in x86_64?

qutim-0.2.0-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

qutim-0.2.0-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

qutim-0.2.0-4.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/qutim-0.2.0-4.fc12

qutim-0.2.0-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

qutim-0.2.0-4.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

The spec could be more readable if it was formatted properly. Please use line breaks at 80 charakters, especially in the descriptions. Otherwise they are hard to read on a terminal or in the PackageKit UI.

The name of the tag is "URL", not "Url".

The group of the -devel package should be Development/Libraries instead of Applications/Internet. For the other subpackages you can omit the group tag because it is the same than the one from the base package.

%descriptions should end with a dot. Exclamation marks are even worse than the missing dots.

There are a lot of linguistic and grammar errors in the English descriptions.

The icon cache scriptlets are outdated, please use the latest version from 
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

Omit --add-category="Network" from desktop-file-install because this is already in the file.

Please preserve timestamps during %install, 
install -p -m 644 icons/%{name}.png ...
...
make install DESTDIR="%{buildroot}" INSTALL="install -p"
see https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

Please use %global instead of %define, see 
https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
Just a hint, this is commented out currently.

(In reply to comment #41)
> The spec could be more readable if it was formatted properly. Please use line
> breaks at 80 charakters, especially in the descriptions. Otherwise they are
> hard to read on a terminal or in the PackageKit UI.
Yes, I was nit enouth carefully and in qutim-plugman description some lines exceedes 80 characters. Fixed. Thanks.
> 
> The name of the tag is "URL", not "Url".
Is it have any sence?

> The group of the -devel package should be Development/Libraries instead of
> Applications/Internet.
Thanks, fixed.

> For the other subpackages you can omit the group tag
> because it is the same than the one from the base package.
It was required before in each defined package. Some changed nowadays?

> %descriptions should end with a dot. Exclamation marks are even worse than the
> missing dots.
I'm do not see any worth in dot, especially in one line descriptions, but it is not bad idea in any case. Ok.

> There are a lot of linguistic and grammar errors in the English descriptions.
Ohh, yes, I known. I recheck it again. And if you can point to concrete mistakes I'll fix it with pleasure.

> The icon cache scriptlets are outdated, please use the latest version from 
> https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
Updated.

> Omit --add-category="Network" from desktop-file-install because this is already
> in the file.
Ok.

> 
> Please preserve timestamps during %install, 
> install -p -m 644 icons/%{name}.png ...
> ...
> make install DESTDIR="%{buildroot}" INSTALL="install -p"
> see https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
Yes, this is my mistake. Thanks. Fixed.

> Please use %global instead of %define, see 
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
> Just a hint, this is commented out currently.    
Yes, it is not used now. BTW I replace it.


http://koji.fedoraproject.org/koji/taskinfo?taskID=2003770