[Bug 785923] Review Request: xgap - GUI for GAP

bugzilla at redhat.com bugzilla at redhat.com
Fri Feb 17 23:09:01 UTC 2012


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=785923

--- Comment #6 from Jerry James <loganjerry at gmail.com> 2012-02-17 18:09:00 EST ---
(In reply to comment #4)
> The desktop file has a number of flaws. I therefore attached a modified copy.

Thank you.

> - Don't hardcode icons; I also wonder why this package has no icon but there is
> an icon and a copy of this desktop file in gap-core, which is no desktop
> application, as far as I can see.

The icon comes with the GAP upstream distribution.  The desktop file for GAP
comes from Debian.  It works, too, due to the "Terminal=true" in the GAP
desktop file.

Since this package is just an X Windows wrapper for GAP, it doesn't have its
own icon, but borrows GAP's.  However, it needs a different desktop file since
it should have "Terminal=false" and a different executable name.

> - Is there really a MIME type? If so, you need a scriptlet.

The MIME type is defined by the gap-core package.  The main GAP executable is
already registered for that mime type, so this desktop file gives users a
second choice of program to use to open such files.

> - Would you really execute xgap handing over a list of URLs? (%U)

Eek.  No, that should be %f.  Thanks for catching that.

> - "Comment" is shown as context help for the application and should give the
> user a clue what that program does. You don't need a comment, but the original
> comment is not suitable.

Thanks, I've adopted your suggestion for this.

> Please use the name macro consistently. You're using it on some occasions, but
> not on others, e. g. patch0.

Fixed.

> Please always change the release number and write to the changelog. The
> reviewer otherwise can easily miss out changes.

Sorry, I usually do that, but got into too much of a rush with this update and
forgot until I had already uploaded the packages.  It won't happen again.

> I wonder if everything installed in /usr/share/gap/pkg/xgap is really
> necessary, for instance manual.dvi, manual.tex or Makefile. The same is true
> for other gap packages. You might also consider to install the necessary files
> with the doc macro and leave a link.

When I was working on the main GAP package, I tried dropping various files out
of doc.  No matter what I omitted, it made some part of the internal document
viewer stop working.  Even the source files are used, by an internal indexer. 
Even the READMEs are used, by an internal README viewer!  I couldn't find
anything that I could leave out without breaking something, so I gave up and
left them all in.  The same seems to be true for the GAP packages.

I'll experiment with using %doc and a link.  It makes me a little nervous, I'll
admit, since my other experiments worked out so badly, but I'll try.

Minus using %doc, all the other changes you requested are here:

Spec URL: http://jjames.fedorapeople.org/xgap/xgap.spec
SRPM URL: http://jjames.fedorapeople.org/xgap/xgap-4.21-2.fc16.src.rpm

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



More information about the package-review mailing list