[Bug 434973] Review Request: scidavis

[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: scidavis


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





------- Additional Comments From lkundrak at redhat.com  2008-03-08 07:45 EST -------
1.) URLs from download from sourceforge

Please use "http://download.sourceforge.net/sourceforge/..." instead of a
specific mirror.

Source0:
http://dfn.dl.sourceforge.net/sourceforge/scidavis/%{name}-%{version}.tar.bz2
Source1:
http://dfn.dl.sourceforge.net/sourceforge/scidavis/scidavis-0.1.2_translations_2008-02-03.tar.bz2
Source2:
http://dfn.dl.sourceforge.net/sourceforge/scidavis/scidavis-manual-0.1_2008-02-28.tar.bz2

2.) Try to specify an URL for these:

Source5:	application-x-scidavis.svg
Source6:	application-x-scidavis-32x32.png
Source7:	application-x-scidavis-48x48.png
Source8:	application-x-scidavis-128x128.png

Or at least a comment where did you get those.
Is it needed to include the pngs?

3.) Correct the names of the patches:

Patch0:		scidavis-translations.patch
Patch1:		scidavis-pro.patch
Patch2:		scidavis-manual.patch

name-version-what.patch; where version is the version you generated those against

4.) Is this needed?

%package manual
...
Requires:	%{name}

Why does manual depend on the package?

5.) X-Fedora category is deprecated, no?

--add-category X-Fedora \

6.) Does this work?

Source1:
http://dfn.dl.sourceforge.net/sourceforge/scidavis/scidavis-0.1.2_translations_2008-02-03.tar.bz2
..
tar -xf %{SOURCE1} -C %{buildroot}%{_datadir}/%{name}/

Is source1 really a bzipped tarball? Why don't you unpack it with -j?


7.) Handle documentation properly.

Don't do this. Use %doc in %files.

install -d %{buildroot}%{_datadir}/doc/%{name}-%{version}/
tar -xf %{SOURCE2} -C %{buildroot}%{_datadir}/doc/%{name}-%{version}/
install -pm 644 CHANGES %{buildroot}%{_datadir}/doc/%{name}-%{version}/
install -pm 644 README %{buildroot}%{_datadir}/doc/%{name}-%{version}/
install -pm 644 gpl.txt %{buildroot}%{_datadir}/doc/%{name}-%{version}/

It also needs some more work in %files regarding files in %doc

8.) Don't these overlap?

%{_libdir}/scidavis/
%{_libdir}/scidavis/plugins/*

and

%{_datadir}/icons/hicolor/scalable/mimetypes/application-x-scidavis.svg
%{_datadir}/icons/hicolor/*/mimetypes/application-x-scidavis*


I will continue the review once you address these.

Thanks!

-- 
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, or are watching someone who is.