[Bug 165616] Review Request: An advanced twin-panel (commander-style) file-manager for KDE

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: An advanced twin-panel (commander-style) file-manager for KDE


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


gajownik at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gajownik at gmail.com




------- Additional Comments From gajownik at gmail.com  2005-08-11 17:11 EST -------
Hi!

There's a small problem - this package does not compile on my machine:

if g++ -DHAVE_CONFIG_H -I. -I. -I../.. -I/usr/include/kde
-I/usr/lib/qt-3.3/include -I/usr/X11R6/include   -DQT_THREAD_SUPPORT 
-D_REENTRANT -D_LARGEFILE64_SOURCE -DKDE_NO_COMPAT -DQT_NO_ASCII_CAST 
-Wnon-virtual-dtor -Wno-long-long -Wundef -ansi -D_XOPEN_SOURCE=500
-D_BSD_SOURCE -Wcast-align -Wconversion -Wchar-subscripts -Wall -W
-Wpointer-arith -Wwrite-strings -O2 -O2 -g -pipe -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -m32 -march=i386 -mtune=pentium4 -fasynchronous-unwind-tables
-Wformat-security -Wmissing-format-attribute -fno-exceptions -fno-check-new
-fno-common  -MT kraddbookmarkdlg.o -MD -MP -MF ".deps/kraddbookmarkdlg.Tpo" -c
-o kraddbookmarkdlg.o kraddbookmarkdlg.cpp; \
then mv -f ".deps/kraddbookmarkdlg.Tpo" ".deps/kraddbookmarkdlg.Po"; else rm -f
".deps/kraddbookmarkdlg.Tpo"; exit 1; fi
../Panel/listpanel.h:152: error: ISO C++ forbids declaration of 'ListPanelFunc'
with no type
../Panel/listpanel.h:152: error: expected ';' before '*' token
../Panel/panelfunc.h: In member function 'ListPanelFunc*
ListPanelFunc::otherFunc()':
../Panel/panelfunc.h:85: error: 'class ListPanel' has no member named 'func'
../krslots.h: In member function 'void KRslots::syncPanels()':
../krslots.h:121: error: 'class ListPanel' has no member named 'func'

and at the end compilation fails. Have you tested your package on FC4? With
which compiler? I've got this one:

[y4kk0 at X devel]$ rpm -q gcc-c++
gcc-c++-4.0.1-4.fc4
[y4kk0 at X devel]$

My (partial) review:
- Summary should not end with dot. You should use rpmlint to be informed about
such a mistakes → http://fedoraproject.org/wiki/PackagingGuidelines#rpmlint
- please provide full URL to the Source0 file:
Source0:        http://prdownloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
- missing BuildRequires: automake
Have you run fedora-rmdevelrpms program?
http://fedoraproject.org/wiki/PackagingGuidelines#BuildRequires
- Since FC3, Fedora has been using gamin instead of fam. Maybe it would be
better to change fam-devel to gamin-devel.
- %setup -q -n %{name}-%{version} ← you don't have to manually specify "-n"
option. It's not a bug but IMHO it's better to have less commands in spec file.
 easy to read = less bugs ;]
- install *.desktop files using desktop-file-install:
http://fedoraproject.org/wiki/Extras/FedoraDesktopEntryGuidelines

I wasn't able to test it but this should be okey:
desktop-file-install --vendor fedora \
  --dir $RPM_BUILD_ROOT%{_datadir}/applications \
  --add-category X-Fedora \
  --delete-original \
  $RPM_BUILD_ROOT%{_datadir}/appl*/*/*.desktop

I think that you can also drop "X-Red-Hat-Base" from patch file.

- "%{_mandir}/man1/krusader.1.gz" change to "%{_mandir}/man1/krusader.1*" on
some systems man pages are not compressed or compressed with bzip2.

On fedora.pl forum people had problems with compiling krusader on 64-bit
systems. You may need to use this workaround →
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=161343#c3

I wasn't able to check anything else :/

Oh, and the last thing. Would you be willing to change the order of the lines in
specfile? It's not need but please compare your spec with the template from
fedora-rpmdevtools package (/usr/share/fedora/spectemplate-minimal.spec). IMHO
it's better to have "Summary:" field after "Name:", "Version:", "Release:" tags.
You may of course not agree with me :D

Thanks,
Dawid

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.