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@xxxxxxxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review