Bug 196120 - Review Request: gresistor
Review Request: gresistor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-06-21 04:38 EDT by Chitlesh GOORAH
Modified: 2010-07-19 23:10 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-05 10:42:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Chitlesh GOORAH 2006-06-21 04:38:18 EDT
Spec URL: http://beta.glwb.info/gresistor/gresistor.spec
SRPM URL: http://beta.glwb.info/gresistor/gresistor-0.0.1-1.src.rpm
Description: <description here>
To allow for identification, resistors are usually marked with
colored bands. Often refereed to as color codes, these markings
are indicative of their resistance, tolerance and temperature 
coefficient. gResistror is a great program that will help you 
translate a resistor color codes into a readable value.
Comment 1 Jeffrey C. Ollie 2006-06-21 12:32:35 EDT
A few notes:

1) BuildArchitectures => BuildArch
2) The "mkdir" lines in the %install section are superfluous.
3) There are no packages GTK+, glade or pygtk in Fedora.  The package builds,
but will not install.  Makes me wonder if package installation was tested.
4) Requires should probably be pygtk2 and pygtk2-libglade.
Comment 3 Parag AN(पराग) 2006-06-22 01:00:19 EDT
I am not able to download Updated SRPM.
Comment 4 Chitlesh GOORAH 2006-06-22 05:20:02 EDT
Fixed and Updated.
Spec URL: http://beta.glwb.info/gresistor/gresistor.spec
SRPM URL: http://beta.glwb.info/gresistor/gresistor-0.0.1-2.src.rpm
Comment 5 Parag AN(पराग) 2006-06-22 05:54:04 EDT
Not an official review as I'm not yet sponsored

Mock build for i386 development gave
/var/tmp/gresistor-0.0.1-2.fc6-root-mockbuild/usr/share/applications/gresistor.desktop:
key "Categories" string list not semicolon-terminated, fixing
   You Need to add semicolon at end of Categories string list

MUST Items:
     - MUST: rpmlint shows no error 
     - MUST: dist tag is present
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package gresistor, in the
format gresistor.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license GPL.
      - MUST: The License field in the package gresistor.spec file did NOT match
any file in tarball.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct.
      - MUST: This package did NOT owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package  have a %clean section, which contains rm -rf
$RPM_BUILD_ROOT.
      - MUST: This package used macros.
      - MUST: Document files are included like README.
      - MUST: This Package include a gresistor.desktop file, and that file is 
installed with desktop-file-install in the %install section successfully.
      - MUST: Package is calling ldconfig on postun post successfully.
      * Source URL is present.
      * BuildRoot is correct BuildRoot:       
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
      * BuildRequires is correct
      * Able to see Desktop icon along with its icon

You need to 
      * Add license file in tarball
      * Make package belong to /usr/share/gresistor under %files as
       %dir %{python_sitelib}/gresistor
Comment 6 Jeffrey C. Ollie 2006-06-22 09:57:28 EDT
The "gtk+" package that you require in the -2 version is GTK+ v1.2.10.  The
installation of this package works because the gtk+ packages in Fedora have an
epoch set, so rpm/yum is really comparing the 1:1.2.0 epoch/version of the gtk+
package vs the 0:2.2 epoch/version of the requires line in the gresistor
package.  In any case, we do not want to pull in GTK+ 1.x when it's GTK+ 2.x
that is really needed.  Just drop the gtk+ dependency, the pygtk2-libglade
dependency should be  sufficient.

Also, the licensing should be cleared up.  The only evidence of a license that I
could find was a "license = 'GPL'" setting in setup.py.  Also, the
SimpleGladeApp.py file seems to be licensed LGPL by a separate author.

A minor nit, but "resistor" was misspelled in the description.
Comment 7 Chitlesh GOORAH 2006-07-01 10:42:24 EDT
Fixed and Updated.
Spec URL: http://beta.glwb.info/gresistor/gresistor.spec
SRPM URL: http://beta.glwb.info/gresistor/gresistor-0.0.1-3.src.rpm

Upstream has been informed for the licence, but hasn't yet replied.
I'm waiting upstream's release.
Comment 8 Till Maas 2006-08-24 16:19:08 EDT
Cannot access specfile:

$ curl -I http://beta.glwb.info/gresistor/gresistor.spec
curl: (7) couldn't connect to host
Comment 10 Christoph Wickert 2006-08-25 11:31:43 EDT
Files section still wrong: Package needs to own %dir %{_datadir}/%{name}

*.pyo files should no longer be ghosted, see
http://fedoraproject.org/wiki/Packaging/Python#head-e48d83dfeb5e671e2018d361d6e75d7e6c6e519c
Comment 11 Chitlesh GOORAH 2006-08-25 14:07:21 EDT
Updated
http://chitlesh.funpic.de/rpm/gresistor.spec
http://chitlesh.funpic.de/rpm/gresistor-0.0.1-5.src.rpm

before, I forgot to increment from 3 to 4, that's why we are at 5 now :)
Comment 12 Mamoru TASAKA 2006-09-02 14:15:11 EDT
Well, functionally good.

I will review this.
Comment 13 Mamoru TASAKA 2006-09-03 09:17:21 EDT
First review of gresistor:

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

   * Licensing
     - Well, it seems that this package is distributed under
     GPL (my recognition is that GPL is more strict than LGPL,
     so if the package includes the code of both GPL and LGPL,
     the license of the whole package is GPL, perhaps).
     However, it would be better that you ask for upstream to
     clarify the license (from the discussion above, it seems
     you have already did it).

   * Requires:
     - python <- required by pygtk2-libglade
     Also, this package requires python(abi) = 2.4.

   * Compiler flags
     - Well, usually CFLAGS="$RPM_OPT_FLAGS" is needed, however,
     how about for this package? This src package don't have any .c
     files and this is a NOARCH package, so CFLAGS should not be
     necessary.

2. From http://fedoraproject.org/wiki/PackagingDrafts/ScriptletSnippets :

   * Requires(post,postun)
     - Well, all of Requires(post,postun) seems unnecessary accoding to
     the URL above. (%post, %postun scriptlets are necessary),

   * GTK+ icon cache
     - No icons are installed under /usr/share/icons. Perhaps it is
     better that
     + create symlink under /usr/share/icons/hicolor/48x48/apps which
       points to /usr/share/gresistor/icon.png
     + fix (fedora-)gresistor.desktop
     + and call gtk-update-icon-cache

3. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :

   = Nothing.

4. Other things I have noticed:

   = Nothing.
Comment 14 Chitlesh GOORAH 2006-09-04 06:57:19 EDT
(In reply to comment #13)
> First review of gresistor:
> 
> 1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> 
>    * Licensing
>      - Well, it seems that this package is distributed under
>      GPL (my recognition is that GPL is more strict than LGPL,
>      so if the package includes the code of both GPL and LGPL,
>      the license of the whole package is GPL, perhaps).
>      However, it would be better that you ask for upstream to
>      clarify the license (from the discussion above, it seems
>      you have already did it).

Ok, I've opted for GPL

>    * Requires:
>      - python <- required by pygtk2-libglade
>      Also, this package requires python(abi) = 2.4.

Fixed

>    * Compiler flags
>      - Well, usually CFLAGS="$RPM_OPT_FLAGS" is needed, however,
>      how about for this package? This src package don't have any .c
>      files and this is a NOARCH package, so CFLAGS should not be
>      necessary.

Fixed

> 2. From http://fedoraproject.org/wiki/PackagingDrafts/ScriptletSnippets :
> 
>    * Requires(post,postun)
>      - Well, all of Requires(post,postun) seems unnecessary accoding to
>      the URL above. (%post, %postun scriptlets are necessary),

Fixed

>    * GTK+ icon cache
>      - No icons are installed under /usr/share/icons. Perhaps it is
>      better that
>      + create symlink under /usr/share/icons/hicolor/48x48/apps which
>        points to /usr/share/gresistor/icon.png
>      + fix (fedora-)gresistor.desktop
>      + and call gtk-update-icon-cache
> 

Actually it already installs its own png at /usr/share/gresistor/icon.png, the
use of GTK+ icon cache and touch --no-create %{_datadir}/icons/hicolor was to
update gnome/kde menus just after the install of gresistor.

Hence a kde/gnome restart is not required to update the icons in the gnome/kde
menus.


Updated
http://chitlesh.funpic.de/rpm/gresistor.spec
http://chitlesh.funpic.de/rpm/gresistor-0.0.1-6.src.rpm
Comment 15 Mamoru TASAKA 2006-09-04 07:55:23 EDT
(In reply to comment #14)
 
> Actually it already installs its own png at /usr/share/gresistor/icon.png, the
> use of GTK+ icon cache and touch --no-create %{_datadir}/icons/hicolor was to
> update gnome/kde menus just after the install of gresistor.
> 
> Hence a kde/gnome restart is not required to update the icons in the gnome/kde
> menus.

Well, my opinition is that if this package uses a icon under
/usr/share/gresistor, not under /usr/share/icons/hicolor, then
touching /usr/share/icons/hicolor or recreating cache under 
/usr/share/icons/hicolor is not necessary.
Comment 16 Chitlesh GOORAH 2006-09-04 08:28:58 EDT
Updated
http://chitlesh.funpic.de/rpm/gresistor.spec
http://chitlesh.funpic.de/rpm/gresistor-0.0.1-7.src.rpm

I've opted for:
* GTK+ icon cache
     - No icons are installed under /usr/share/icons. Perhaps it is
     better that
     + create symlink under /usr/share/icons/hicolor/48x48/apps which
       points to /usr/share/gresistor/icon.png
     + fix (fedora-)gresistor.desktop
     + and call gtk-update-icon-cache
Comment 17 Mamoru TASAKA 2006-09-04 10:08:10 EDT
Well, I think that the name of "icon.png" is somewhat
troublesome. Perhaps using "gresistor.png" is better to
aboid comflict from other packages.
Comment 19 Mamoru TASAKA 2006-09-04 10:56:28 EDT
Okay. No problem.

I am pleased to say that this package (gresistor) is

  APPROVED

by me.
Comment 20 Chitlesh GOORAH 2010-07-18 06:39:06 EDT
Package Change Request
=======================
Package Name: gresistor
Short Description: GGnome resistor color code calculator
Owners: chitlesh
Branches: EL-5 EL-6
Comment 21 Kevin Fenzi 2010-07-19 00:24:15 EDT
cvs done.
Comment 22 Jason Tibbitts 2010-07-19 12:37:30 EDT
The misformatted nature of the CVS request ("Branches" instead of "New Branches") confused the processing script a bit.  Please ignore flag changes while I test out a fix.
Comment 23 Kevin Fenzi 2010-07-19 23:10:49 EDT
Setting this back to let me process the rest of the queue. ;)

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