This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 218172 - Review Request: gnome-chemistry-utils - A set of chemical utilities
Review Request: gnome-chemistry-utils - A set of chemical utilities
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: 218210
Blocks: FE-ACCEPT 218176
  Show dependency treegraph
 
Reported: 2006-12-02 10:29 EST by Julian Sikorski
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-06 14:20:26 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Julian Sikorski 2006-12-02 10:29:41 EST
Spec URL: http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils.spec
SRPM URL: http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils-0.6.3-1.src.rpm
Description: <description here>
This package is a set of chemical utils. Three programs are avaible:
* A 3D molecular structure viewer (GChem3Viewer).
* A Chemical calculator (GChemCalc).
* A periodic table of the elements application (GChemTable).

Package builds fine inside mock, and the only warnings that rpmlint returns are lacks of documentation for devel and mozplugin packages. This is my first package (apart from museek+ that still waits for review), so it looks like I need a sponsor.
Comment 1 Mamoru TASAKA 2006-12-02 13:17:03 EST
Well, interesting package.
From a quick view:

A. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* Requires:
  - for -mozplugin package:
    Why does this require pkgconfig?

  - for -devel package:
    Please check /usr/lib/pkgconfig/gcu.pc
----------------------------------------------------------
Requires: libglade-2.0 libgnomeui-2.0 libgnomeprintui-2.2 gtkglext-1.0 openbabel-2.0
----------------------------------------------------------
    This means that this (-devel) package requires:
---------------------------------------------------------
(libglade2-devel : required by libgnomeui-devel so this is redundant).
libgnomeui-devel
libgnomeprintui22-devel
gtkglext-devel
openbabel-devel
---------------------------------------------------------
   Otherwise we get a error like:
---------------------------------------------------------
[tasaka1@localhost ~]$ pkg-config --cflags gcu
Package openbabel-2.0 was not found in the pkg-config search path.
Perhaps you should add the directory containing `openbabel-2.0.pc'
to the PKG_CONFIG_PATH environment variable
Package 'openbabel-2.0', required by 'Gnome Chemistry Utils', not found
---------------------------------------------------------

* Documentation
  - Files under docs/reference/html are rather large in total and
    most (or all?) files are automatically generated by doxygen.
    I think these files should be in -devel package.

* Desktop files
  - Please use 'desktop-file-install' and add 'fedora' as vendor tag (then add
    'desktop-file-utils' to BuildRequires)
  - Would you ask upstream to include icons for GUI desktop?

* Timestamps
  - Well, -devel package contains a lot of header files so
  keeping timestamps is highly preferable as
  * it shows if vendor (like you) have modified the original
    files
  * it shows when the files are created

  So keep timestamps, at least for header files.
  Usually,
--------------------------------------------------------
make INSTALL="install -p" install
--------------------------------------------------------
  plays the trick.

  - /usr/bin/install -c -m 644
    Use "%{__install} -c -p -m 644" to keep timestamps, use
    macros.

B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   (Okay)

C. Other things I have noticed:
*  test -z $RPM_BUILD_ROOT%{_datadir}/mime/packages ||
   - This is redundant because it is apparently not zero length.
Comment 2 Mamoru TASAKA 2006-12-02 13:25:10 EST
Parag, if you don't mind, I want to assign this bug to myself
as a sponsor.
Comment 3 Parag AN(पराग) 2006-12-02 13:43:18 EST
(In reply to comment #2)
> Parag, if you don't mind, I want to assign this bug to myself
> as a sponsor.
Sure Go Ahead. You are always welcome for aa long as its a review for
FE-NEEDSPONSOR. Becuase i don't have sponsor status.
Comment 4 Mamoru TASAKA 2006-12-02 13:45:37 EST
(In reply to comment #3)
> Sure Go Ahead. You are always welcome for aa long as its a review for
> FE-NEEDSPONSOR. Becuase i don't have sponsor status.

Thank you.

Comment 5 Julian Sikorski 2006-12-03 03:46:44 EST
Spec URL: http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils.spec
SRPM URL:
http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils-0.6.3-2.src.rpm

New release: 
- Fixed Requires for -devel and -mozplugin packages
- Moved files in docs/reference/html to the devel package
- Install .desktop files using desktop-file-install
- Removed redundant stuff
- Preserve timestamps

I'll ask upstream for icons. BTW, why some .desktop files need to be installed
using desktop-file-install and some do not (or are all these packages wrong)?
Comment 6 Julian Sikorski 2006-12-03 04:08:43 EST
The icons enquiry has been filed:
https://savannah.nongnu.org/bugs/index.php?18443
Comment 7 Mamoru TASAKA 2006-12-03 04:20:10 EST
Well, before checking 0.6.3-2: (I am currently taking dinner and
I will check it later)

(In reply to comment #5)
> BTW, why some .desktop files need to be installed
> using desktop-file-install and some do not (or are all these packages wrong)?
My recognition is that all packages in Fedora Core/Extras are required
to use desktop-file-utils when installing desktop files. At least
all packages I reviewed are modified to use desktop-file-utils when
it tries to install desktop files without it.

So under my recognition packages installing desktop files without 
desktop-file-utils are all wrong.

NOTE: there are some desktop-*like* files which are installed without
desktop-file-utils (e.g. in xscreensaver-extras-gss)
Comment 8 Julian Sikorski 2006-12-03 06:04:31 EST
This software supports chemical-mime-data, so I'll try to wrap up a package.
Feel free to continue the review in the meantime.
Comment 9 Julian Sikorski 2006-12-03 07:47:02 EST
It was not that hard.
Spec URL: http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils.spec
SRPM URL:
http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils-0.6.3-1.src.rpm
New release:
- Added doxygen to BuildRequires
- Added support for chemical-mime-data
Now it is blocked by bug #218210
Comment 10 Julian Sikorski 2006-12-03 07:47:43 EST
Oops...
SRPM URL:
http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils-0.6.3-3.src.rpm
Comment 11 Mamoru TASAKA 2006-12-03 10:46:15 EST
Well:
* BuildRequires:  chemical-mime-data
  Should this be 'BuildRequires' or 'Requires'?
* By the way, please remove unnecessary comments like:
------------------------------------------------
#mkdir -p -- $RPM_BUILD_ROOT%{_datadir}/mime/packages
------------------------------------------------
  which are no longer used to make spec file more legible.

Note:
--add-category=X-Fedora
  Well, FE packaging committee made a semi(?)-conclusion that
  this should be removed because
* desktop-file-utils 0.11 does _NOT_ allow this so
* anyway this is deprecated and of no use
  However, now rawhide is desktop-file-utils 0.12 and this again
  _ALLOWS_ --add-category=X-Fedora ......

I think that '--add-category=X-Fedora' should be removed until
the argument that "--add-category=X-Fedora should be revived"
arises, however, it seems that this (--add-category=X-Fedora) is
okay for now??
Comment 12 Julian Sikorski 2006-12-03 14:43:58 EST
Spec URL: http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils.spec
SRPM URL:
http://www.republika.pl/belegdol/rpmstuff/gnome-chemistry-utils-0.6.3-4.src.rpm
New release:
- Removed obsolete stuff
- Fixed support for chemical-mime-data
- Removed --add-category X-Fedora from desktop-file-install command

I think that chemical-mime-data should be present both in BuildRequires and in
Requires, as it is checked for presence by configure (and probably should be
installed for this whole operation to make sense).
Comment 13 Mamoru TASAKA 2006-12-04 02:44:28 EST
Okay.

---------------------------------------------
  This package (gnome-chemistry-utils) is APPROVED by me.
Comment 14 Mamoru TASAKA 2006-12-04 20:55:44 EST
Removing FE-NEEDSPONSOR (bug 218210)
Comment 15 Julian Sikorski 2006-12-05 03:19:14 EST
Thanks very much for sponsorship. Package imported, FC-5 and FC-6 branches
requested.

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