Bug 818805 - Review Request: openerp-client - Business Applications Server Client
Review Request: openerp-client - Business Applications Server Client
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Brendan Jones
Fedora Extras Quality Assurance
:
Depends On: 817270
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 23:51 EDT by Alec Leamas
Modified: 2012-06-21 14:13 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-06-21 14:02:43 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
brendan.jones.it: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Alec Leamas 2012-05-03 23:51:38 EDT
Spec URL: http://dl.dropbox.com/u/17870887/python-spiffgtkwidgets-0.2.0-2/python-spiffgtkwidgets.spec
SRPM URL: http://dl.dropbox.com/u/17870887/python-spiffgtkwidgets-0.2.0-2/python-spiffgtkwidgets-0.2.0-2.59a713f.fc16.src.rpm

Description:
Gtk client for Open ERP.

OpenERP is a free Enterprise Resource Planning and Customer Relationship
Management software. It is mainly developed to meet changing needs.

This package only contains the thin, native client for the ERP application.
After installing this, you will be able to connect to any OpenERP server
running in your local network or the Internet.

rpmlint openerp-client-6.1-1.fc18.noarch.rpm

openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
- Package actually requires it, and it's not picked up automagically.
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

$ rpm -q --requires openerp-client | egrep -v 'rpmlib|bin/sh' | uniq
    desktop-file-utils
    hippo-canvas-python
    mx
    pydot
    pygobject2
    pygtk2
    pygtk2-libglade
    python(abi) = 2.7
    python-dateutil
    python-lxml
    python-spiffgtkwidgets
$ rpm -q --provides openerp-client | egrep -v 'rpmlib|bin/sh' | uniq
    openerp-client = 6.1-1.fc16
$ rpmlint openerp-client
    openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
    1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Comment 1 Alec Leamas 2012-05-03 23:54:03 EDT
Updating metadata
Comment 3 Brendan Jones 2012-06-13 09:08:52 EDT
I will take this review.

(Alex can you please use you fedorapeople.org account for hosting your specs/SRPMS?)
Comment 4 Alec Leamas 2012-06-13 09:50:36 EDT
Sure. Unless it's a problem for you, I'll wait until there is a new set to publish. Let me know if also this is a problem, and I'll fix. Havn't really got used to use fedorapeople.org yet ;)
Comment 5 Alec Leamas 2012-06-13 10:02:36 EDT
Done. Links at http://leamas.fedorapeople.org/openerp-client.
Comment 6 Brendan Jones 2012-06-13 15:42:54 EDT
Hey Alec (apologies - misspelled your name before)

Just a few things to clarify before I dig in to this one.

 - you should prefix your sources with the package name (and version if relevant)
 - when providing an alternate license file you need to explicitly state where you got it from
 - I'm confused by the changelog. Where was this spec from originally? You need to cull the invalid change log entries. A single changelog entry will suffice if the spec was sourced out of Fedora, but even this is not necessary.

 - Rpmlint throws up an error against noarch:

rpmlint /home/bsjones/rpmbuild/SRPMS/openerp-client-6.1-1.fc17.src.rpm /home/bsjones/rpmbuild/RPMS/noarch/openerp-client-6.1-1.fc17.noarch.rpm
openerp-client.src: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

 - use the %{name} macro wherever you can 
 - I like to see all of the docs/man/bin files defined explicitly in the %files section (I find it helps in the review). Its safer, and you can get a better picture of the installed RPM. The suffix for man files should be .* however

Sounds like a lot, but not really. All in pretty good shape
Comment 7 Brendan Jones 2012-06-13 15:45:16 EDT
(In reply to comment #6)
> suffice if the spec was sourced out of Fedora, but even this is not

Sorry *outside* of Fedora
Comment 8 Alec Leamas 2012-06-13 17:31:46 EDT
(In reply to comment #6)
Hi! Here we go:

Things partly fixed in 
spec: http://leamas.fedorapeople.org/openerp-client/openerp-client.spec
srpm: http://leamas.fedorapeople.org/openerp-client/openerp-client-6.1-2.fc16.src.rpm

>  - you should prefix your sources with the package name (and version if
> relevant)
Done for the licensing file.

>  - when providing an alternate license file you need to explicitly state
> where you got it from
There is no alternate license, just a license break-down with a possibly bad name. Renamed, see above

>  - I'm confused by the changelog. Where was this spec from originally? You
> need to cull the invalid change log entries. A single changelog entry will
> suffice if the spec was sourced out of Fedora, but even this is not
> necessary.
So am I. But this request is based on old bug 693425 (which I should have noted in the original request!) The changelog is from there. With this said, noone would be happier than me if we could drop it. Do you think it's OK, given this?

>  - Rpmlint throws up an error against noarch: 
[cut]
> openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
[cut]
This one beats me. I think its a rpmlint error, it somehow thinks anything named like mathplotlib is a "normal" lib. However, I really need this Requires:, there is no automatic resolver for python handling this, and I need the python module. 

>  - use the %{name} macro wherever you can
Done (although note that the guidelines explicitly says this is a matter of style).

>  - I like to see all of the docs/man/bin files defined explicitly in the
> %files section (I find it helps in the review). Its safer, and you can get a
> better picture of the installed RPM. The suffix for man files should be .*
> however
Done
Comment 9 Brendan Jones 2012-06-13 17:53:42 EDT
(In reply to comment #8)

> >  - I'm confused by the changelog. Where was this spec from originally? You
> > need to cull the invalid change log entries. A single changelog entry will
> > suffice if the spec was sourced out of Fedora, but even this is not
> > necessary.
> So am I. But this request is based on old bug 693425 (which I should have
> noted in the original request!) The changelog is from there. With this said,
> noone would be happier than me if we could drop it. Do you think it's OK,
> given this?
> 

Wow! war and peace - yes, drop all of the change log - its not relevant.

> >  - Rpmlint throws up an error against noarch: 
> [cut]
> > openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
> [cut]
> This one beats me. I think its a rpmlint error, it somehow thinks anything
> named like mathplotlib is a "normal" lib. However, I really need this
> Requires:, there is no automatic resolver for python handling this, and I
> need the python module. 

You are right. I did a quick BZ search - this has come up a few times.

> 
> >  - use the %{name} macro wherever you can
> Done (although note that the guidelines explicitly says this is a matter of
> style).

True!

> 
> >  - I like to see all of the docs/man/bin files defined explicitly in the
> > %files section (I find it helps in the review). Its safer, and you can get a
> > better picture of the installed RPM. The suffix for man files should be .*
> > however
> Done

Thanks Alec. I'll try and finish this in the next day or so.
Comment 10 Alec Leamas 2012-06-13 18:11:51 EDT
Dropped the changelog. Same links.
Comment 11 Alec Leamas 2012-06-20 07:42:19 EDT
Do I dare to send a ping?
Comment 12 Brendan Jones 2012-06-20 07:44:25 EDT
Apologies Alec, I have been caught up playing host to some friends from home. Should get to this today.
Comment 13 Brendan Jones 2012-06-21 05:38:51 EDT
Just some comments in the Requires: that need dropping. This package is APPROVED


Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST The spec file handles locales properly.
[x]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Requires correct, justified where necessary.
[!]: MUST Rpmlint output is silent.

rpmlint openerp-client-6.1-2.fc18.src.rpm

openerp-client.src: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint openerp-client-6.1-2.fc18.noarch.rpm

openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/bsjones/rpmbuild/SRPMS/openerp-client-6.1-1.tar.gz :
  MD5SUM this package     : f0d349bfa0f1d8979b36a64bd5204b0c
  MD5SUM upstream package : f0d349bfa0f1d8979b36a64bd5204b0c

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[-]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.
[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD Scriptlets must be sane, if used.
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop
[!]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
[!]: MUST Rpmlint output is silent.

rpmlint openerp-client-6.1-2.fc18.src.rpm

openerp-client.src: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint openerp-client-6.1-2.fc18.noarch.rpm

openerp-client.noarch: E: explicit-lib-dependency python-matplotlib
openerp-client.noarch: W: spelling-error %description -l en_US Gtk -> Gk, Gt, Gt k
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint


Generated by fedora-review 0.1.3
External plugins:
Comment 14 Alec Leamas 2012-06-21 11:34:19 EDT
Thanks for review!


New Package SCM Request
=======================
Package Name: openerp-client
Short Description: Business Applications Server Client
Owners: leamas
Branches: f16 f17
InitialCC:
Comment 15 Gwyn Ciesla 2012-06-21 11:42:05 EDT
Brendan, please ser fedora-review to +.
Comment 16 Brendan Jones 2012-06-21 11:57:00 EDT
Apologies!
Comment 17 Alec Leamas 2012-06-21 12:01:48 EDT
You shouldn't apologize, I should. It was I who somehow reset your '+' to '?' when setting fedora-cvs flag.

I have yet to make a cvs request without making a mistake. Seems utterly hard.

New Package SCM Request
=======================
Package Name: openerp-client
Short Description: Business Applications Server Client
Owners: leamas
Branches: f16 f17
InitialCC:
Comment 18 Gwyn Ciesla 2012-06-21 12:09:22 EDT
Git done (by process-git-requests).
Comment 19 Alec Leamas 2012-06-21 14:02:43 EDT
Build OK for rawhide, f17, f16. Closing
Comment 20 Gwyn Ciesla 2012-06-21 14:13:25 EDT
Duplicate.

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