[Bug 199029] Review Request: jokosher

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]