[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-09 06:51 EST -------
(In reply to comment #5)
> (In reply to comment #4)
> > 1.) URLs from download from sourceforge
> Ok

Thanks, the changes are fine.

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

This still doesn't seem correct to me. Is this the file?
http://scidavis.svn.sourceforge.net/viewvc/*checkout*/scidavis/trunk/icons/scidavis-icon.svg?revision=709

If yes, please either specify it in Source: and rename when installing it, or at
least put a comment above it.

And the pngs -- are they needed? When it comes to icons; SVG is generally
sufficient.

> > 3.) Correct the names of the patches:
> Ok

Thanks.

> > 4.) Is this needed?
> > Why does manual depend on the package?
> 
> You are right. Deleted.

Thanks.

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

I see it vanished; thanks.

> > 6.) Does this work?
> > Is source1 really a bzipped tarball? Why don't you unpack it with -j?
> 
> Yes this works but i added the -j

Wow, I would never say it would; if not on FreeBSD with libarchive :o)

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

How about untarring it in %prep after %setup of Source0?
Either just untar it to directory where you are after it, or make some
constructive use of %setup macro. It can take a variety of useful, yet somewhat
tricky options: see [1].

[1] http://www.rpm.org/max-rpm/s1-rpm-inside-macros.html

> > 8.) Don't these overlap?
> You are right!

Ok.

Thanks for your improvements to the spec file, I'll continue the review once doc
files are properly dealt with. Feel free to ask for help if you need any.

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