[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 eric.tanguy at univ-nantes.fr  2008-03-08 13:44 EST -------
(In reply to comment #4)

For the moment i just update the spec file not the src.rpm file because i have
some more question regarding your comments

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

Ok

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

They come from the svn version. I had some exchange with upstream about how to
handle desktop and mime and this will be incuded in the next version.

> 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

Ok

> 
> 4.) Is this needed?
> 
> %package manual
> ...
> Requires:	%{name}
> 
> Why does manual depend on the package?

You are right. Deleted.

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

Yes this works but i added the -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


This was my biggest problem and i suspect this is because i don't do the thing
correctly.
When i put : 
%files
%defattr(-,root,root,-)
%doc CHANGES README gpl.txt
...

i don't know how to handle the tar.bz2 manual in %files manual

So here i need some help

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

You are right!

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