Bug 458457 - Review Request: grc - GUI for GNURadio
Summary: Review Request: grc - GUI for GNURadio
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 460104
TreeView+ depends on / blocked
 
Reported: 2008-08-08 15:38 UTC by Marek Mahut
Modified: 2008-08-31 15:38 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-08-31 15:38:54 UTC
Type: ---
Embargoed:
lkundrak: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Marek Mahut 2008-08-08 15:38:22 UTC
Spec URL: http://mmahut.fedorapeople.org/reviews/grc/grc.spec
SRPM URL: http://mmahut.fedorapeople.org/reviews/grc/grc-0.70-1.fc8.src.rpm

Description: The GNU Radio Companion is a preliminary graphical user interface
which allows GNU Radio components to be put together graphically.
It is currently under development by Josh Blum. GNU Radio Companion
encompasses over 200 blocks from the GNU Radio Project.

Comment 1 Lubomir Rintel 2008-08-08 16:39:20 UTC
Marek, thanks for the package.
Actual source file matches upstream.
Mock rebuild succeeded.
Offered functionality was tested.
RPMlint is mostly silent, see below
Uses macros consistently.

Have you thought about renaming the package to gnuradio-companion? That may express the relation to gnuradio, and you'll prevent a potential name clash. It even comes from the same upstream. Probably one day there'll be a lot of grc packages around :)

0.) RPMLint thinks the package should be noarch

You do not ship any binaries. I do not know if the precompiled python stuff is arch-independent -- probably worth trying.

Also, rpmlint complains about shebangs in nonexecutable files.

1.) Please do not create scripts inline in SPEC

That is utterly ugly. You should be ashamed.

2.) Fix your runtime dependencies.

You seem to import at least wx, gtk, pango, etc. We do not have autoreqs for python. Please add the necessary Requires.

The upstream web [1] recommends the following, which you may want to base your decision upon:

    * wx-python
    * numpy
    * gnuradio with gr-wxgui
    * python-gtk2(>=2.6)
    * python-xml and/or pyxml 

[1] http://gnuradio.org/trac/wiki/GNURadioCompanion

3.) License tag does not seem correct.

At least you include some CC blurb in %doc. Are you sure you don't ship any Creative Commons stuff?

Comment 2 Marek Mahut 2008-08-08 19:14:07 UTC
(In reply to comment #1)
> Marek, thanks for the package.
> Actual source file matches upstream.
> Mock rebuild succeeded.
> Offered functionality was tested.
> RPMlint is mostly silent, see below
> Uses macros consistently.
> 
> Have you thought about renaming the package to gnuradio-companion? That may
> express the relation to gnuradio, and you'll prevent a potential name clash. It
> even comes from the same upstream. Probably one day there'll be a lot of grc
> packages around :)
> 
> 0.) RPMLint thinks the package should be noarch
> 
> You do not ship any binaries. I do not know if the precompiled python stuff is
> arch-independent -- probably worth trying.

You're right, this package should be noarch, will be fixed in next revision.

> Also, rpmlint complains about shebangs in nonexecutable files.
> 
> 1.) Please do not create scripts inline in SPEC
> 
> That is utterly ugly. You should be ashamed.

Any guideline against this? I like it.

> 2.) Fix your runtime dependencies.
> 
> You seem to import at least wx, gtk, pango, etc. We do not have autoreqs for
> python. Please add the necessary Requires.
> 
> The upstream web [1] recommends the following, which you may want to base your
> decision upon:
> 
>     * wx-python
>     * numpy
>     * gnuradio with gr-wxgui
>     * python-gtk2(>=2.6)
>     * python-xml and/or pyxml 
> 
> [1] http://gnuradio.org/trac/wiki/GNURadioCompanion

All those are fetched with GNURadio itself, this should be fine.

> 3.) License tag does not seem correct.
> 
> At least you include some CC blurb in %doc. Are you sure you don't ship any
> Creative Commons stuff?

Only the icon is under CC, however it's shipped with the package under GPLv2+, should I mention it anyway?

Comment 3 Marek Mahut 2008-08-08 19:19:26 UTC
(In reply to comment #1)
> Have you thought about renaming the package to gnuradio-companion? That may
> express the relation to gnuradio, and you'll prevent a potential name clash. It
> even comes from the same upstream. Probably one day there'll be a lot of grc
> packages around :)

It's not from the same upstream, only the guide is hosted on their wiki. It's a separate utility to create configuration files for GNURadio. Thus I know this is not first grc software that will be in Fedora, it's certainly first software w/ "grc" name :)

Comment 5 manuel wolfshant 2008-08-08 22:17:48 UTC
I think that grc is a bit short for a name, too.  Lubomir's suggestion to use gnuradio-companion instead would certainly be useful in preventing name clashes. Just because it's the first piece of software claiming a 3 letter name does not mean it really should do it. Especially as it has a desktop file to launch it, so not typing in the console is really needed.

Eventually you could create a symlink named grc...

Comment 6 Lubomir Rintel 2008-08-08 23:05:30 UTC
> > 1.) Please do not create scripts inline in SPEC
> > 
> > That is utterly ugly. You should be ashamed.
> 
> Any guideline against this? I like it.

Yup:
"- MUST: The spec file for the package MUST be legible."

> 
> > 2.) Fix your runtime dependencies.
> > 
> > You seem to import at least wx, gtk, pango, etc. We do not have autoreqs for
> > python. Please add the necessary Requires.
> > 
> > The upstream web [1] recommends the following, which you may want to base your
> > decision upon:
> > 
> >     * wx-python
> >     * numpy
> >     * gnuradio with gr-wxgui
> >     * python-gtk2(>=2.6)
> >     * python-xml and/or pyxml 
> > 
> > [1] http://gnuradio.org/trac/wiki/GNURadioCompanion
> 
> All those are fetched with GNURadio itself, this should be fine.

I do not think so. Fedora 9:

[lkundrak@jesus ~]$ grc

Missing critical module: "xml.dom.ext"

Exiting!

[lkundrak@jesus ~]$ 

> > 3.) License tag does not seem correct.
> > 
> > At least you include some CC blurb in %doc. Are you sure you don't ship any
> > Creative Commons stuff?
> 
> Only the icon is under CC, however it's shipped with the package under GPLv2+,
> should I mention it anyway?

I do not know what does the CC license of the icon say. By including the icon license in License tag can not do any harm, well, I guess... when in doubt please ask on legal m-l.

Comment 7 Marek Mahut 2008-08-09 07:42:17 UTC
(In reply to comment #6)
> Yup:
> "- MUST: The spec file for the package MUST be legible."

It is!
 
> I do not think so. Fedora 9:
> 
> [lkundrak@jesus ~]$ grc
> 
> Missing critical module: "xml.dom.ext"
> 
> Exiting!
> 
> [lkundrak@jesus ~]$ 

As you said it's PyXML.

> I do not know what does the CC license of the icon say. By including the icon
> license in License tag can not do any harm, well, I guess... when in doubt
> please ask on legal m-l.

You're right done.

Comment 8 Marek Mahut 2008-08-09 07:47:02 UTC
(In reply to comment #5)
> I think that grc is a bit short for a name, too.  Lubomir's suggestion to use
> gnuradio-companion instead would certainly be useful in preventing name
> clashes. Just because it's the first piece of software claiming a 3 letter name
> does not mean it really should do it. Especially as it has a desktop file to
> launch it, so not typing in the console is really needed.

Everyone knows it as grc, it's upstream official name... first anyone would do is to try it install under this name (I could use Provides: grc to save it).

> Eventually you could create a symlink named grc...

But if I create the symlink named grc and provides to grc, that's the point to not name the package itself grc?

Comment 10 Lubomir Rintel 2008-08-26 09:11:53 UTC
Seems mostly fine, thanks!

Please add "$@" to the launch script and remove .png extension of the Icon in desktop files before commiting.

APPROVED

Comment 11 Lubomir Rintel 2008-08-26 09:12:57 UTC
I see you were faster! Thanks^2 :)

Comment 12 Marek Mahut 2008-08-26 09:25:39 UTC
Thank you Lubomir!

New Package CVS Request
=======================
Package Name: grc
Short Description: GUI for GNURadio
Owners: mmahut
Branches: F-8 F-9 EL-5

Comment 13 Kevin Fenzi 2008-08-26 23:25:22 UTC
cvs done.

Comment 14 Marek Mahut 2008-08-31 15:38:54 UTC
Thank you both! Built and waiting in bodhi! \o/


Note You need to log in before you can comment on or make changes to this bug.