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: jokosher https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199029 ------- Additional Comments From snecklifter@xxxxxxxxx 2007-03-06 20:30 EST ------- (In reply to comment #63) > I've found David's issue and will attach a patch that you can take upstream. As > for getting this in, I don't have time for a complete review but I do have a few > comments. Maybe after you fix these and apply the patch, David can continue to > do the review:: I have submitted your patch upstream Toshio, thank you for that. > * Cosmetic: The tarball you've created is really a .tar file, not a .tar.gz. > Rpm knows how to handle it, however, and also compresses its payload so it's not > strictly necessary to fix this. It would be nice to be accurate when a human > extracts the source rpm and tries to look at the sources, though. So having > jokosher-0.9.tar or actually gzipping the tarball would be appropriate :-) Done. > * A recent addition to the Packaging Guidelines is that for packaging snapshots > you need to show how to recreate the snapshot either in a script that you > include as another Source line or in a comment. ie:: > # This tarball is a snapshot. You can recreate it by doing: > # svn co -r 321 http://svn.jokosher.org/trunk jokosher-0.9 > # tar -czvf jokosher-20070225.snap.tar.gz jokosher-0.9 > > This allows reviewers to easily check that the sources are coming from upstream. Done. > * The BuildArch: noarch is missing from the spec file Fixed. > * You aren't cleaning the buildroot prior to installing (rpmlint warns about this) Fixed. > * You aren't installing the omf file and registering with scrollkeeper within > the %post/%postun in the spec file so the help files won't be found. Fixed. > * You aren't calling update-mime-database or update-desktop-database in the spec > file's %post/%postun so jokosher's mimetype and "mailcap" entries aren't being > created. Fixed. > * You have a raft of unowned directories. As an example, changing your file > entries from this: > %{_datadir}/%{name}/pixmaps/*.png > into this: > %{_datadir}/%{name}/ > > will own the jokosher directory and all of its subdirectories and files. > > Where you cannot do this because you don't want all of the files inside the > directories you can change from this:: > %exclude %{python_sitelib}/Jokosher/Profiler.py > %{python_sitelib}/Jokosher/*.py > %{python_sitelib}/Jokosher/*.pyo > %{python_sitelib}/Jokosher/*.pyc > into this: > %exclude %{python_sitelib}/Jokosher/Profiler.py > %dir %{python_sitelib}/Jokosher > %{python_sitelib}/Jokosher/*.py > %{python_sitelib}/Jokosher/*.pyo > %{python_sitelib}/Jokosher/*.pyc Fixed. > * It looks like you've got the jokosher help in three places: > /usr/share/gnome/help/jokosher, /usr/share/doc/jokosher-0.9/userguide, and > /usr/share/doc/jokosher-0.9/jokosher > > It probably only neds to be in /usr/share/gnome/help/ Fixed. > * You need to use the %find_lang macro to include the *.mo files, not just > include them in the %files section. The way you've currently got it setup, > people won't be able to specify which languages they're interested in when they > install this. Done. > * David's error is coming from the section of setup.py dealing with installing > omf files. However, the whole handling of omf files has issues. Attaching a patch. Thank you for taking the time to respond Toshio. I have run through rpmlint and receive no errors. It also appears to build cleanly under mock development on i386. I will notify as and when your patch is accepted upstream however for the moment the new packages are in the usual location at: http://www.iammetal.co.uk/jokosher Regards Chris -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review