[Bug 693425] Review Request: openerp - OpenERP business application

[bugzilla at redhat.com]

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=693425

--- Comment #43 from Alec Leamas <leamas.alec at gmail.com> 2011-04-26 08:13:26 EDT ---
Easter holiday is over, here we go! First, a server review:

The submission is not complete: there should be a source RPM which is missing.
The source URL in the spec file seems to be invalid. This review is based on
the previous submitted sources + the updated spec file.  Please submit source
rpm + spec file urls next time.

MUST

- rpmlint must be run on the source rpm and all binary rpms... NOK
 + Only 2 packages are checked, should be 4 (2 srpms + 2 noarch rpms)
 + The "Non standard uid/gid" warnings can be ignored.
 + The warning on URL http://www.openerp.com/ seems to be a temporary server
problem.
 + The error on zero-length file (.../office.dtd): Is this file required?
 + Fedora init scripts does normally not enable services by default. See
http://fedoraproject.org/wiki/Packaging/SysVInitScript#.23_chkconfig:_line
 + The changelog version should be fixed. See
http://fedoraproject.org/wiki/PackagingGuidelines#Changelogs

- The spec file name must match the base package %{name}.... NOK
 + The spec file should be named openerp-server.spec; see
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Spec_file_name

- The package must meet the Packaging Guidelines. TBD
 + The dependency list looks incomplete. Samples like user_ldap indicates that
some module dependencies are missing. Also, there are references to pytz and
matplotlib, but these are not required. Datetutil is used (imported), no
dependency on python-dateutil. Some deps listed in setup.py seems not to be
included. An overall review of the dependencies is needed.

- The package must be licensed with a Fedora approved license...: OK

- The License field in the package spec file must match the actual license: NOK
 + No overall copyright clause in README or separate copyright file, acceptable
but "odd".
 + License: field is AGPLv3
 + ./bin/addons/document_ftp/ftpserver is MIT, part of bundled pyftpdlib.
 + ./build/lib/openerp-server/addons/wiki/web/widgets/rss/feedparser.py is MIT.
 + ./bin/addons/resource/faces/* are GPLv2+.
 + ./build/lib/openerp-server/addons/document/dict_tools.py is LGPL 2.0, (C) by
you :)
 + The thunderbird plugin is "OpenERP Public License"
 + 'grep -ir  license . | grep -v Affero | grep -i "version 2"' lists files
which are not AGPLv3, mostly LGPL 2.1+. 
See http://fedoraproject.org/wiki/Packaging:LicensingGuidelines 

- Separate License file must be in %doc...: OK

- The spec file must be written in American English: TBD
  + Being a Swede, I really don't know. Looks fine to me, though :)

- The spec file for the package MUST be legible: OK

- The sources used to build the package must match the upstream source...TBD
   No source submitted.

- The package MUST successfully compile and build into binary rpms...: TBD
   No source submitted.

- All build dependencies must be listed in BuildRequires...TBD
   No source submitted.

- The spec file MUST handle locales properly...: OK

- Packages must NOT bundle copies of system libraries: NOK
  + contains bundled ftpserver (in Fedora: pyftpdlib)
  + contains bundled rml2pdf  (in Fedora: python-trml2pdf)

- A package must own all directories that it creates. OK

- A Fedora package must not list a file more than once in %file lists...: OK

- Permissions on files must be set properly...: OK

- Each package must consistently use macros : NOK
 + The _iconsdir macro is not used in the server spec file, remove definition.
 + _initrddir is deprecated, replaced by _initddir; see
http://fedoraproject.org/wiki/Packaging/SysVInitScript

- The package must contain code, or permissable content: OK

- %doc must not affect the runtime of the application...: OK

- Header files must be in a -devel package OK

- Packages must not own files or directories owned by other packages...OK

- All filenames in rpm packages must be valid UTF-8.: OK

SHOULD:

- Package built on koji/mock. TBD
  No source submitted

- Testing...TBD

- Scriptlets should be sane...: OK

OTHER REMARKS:
- The for BIN in... loop could be written using 'sed -i' in just one line
- Remove setup.cfg together with setup.inf. These files are just not relevant
in a Fedora package.
- The copyright info in debian/copyright is outdated, SpiffGtkWidtgets is now
AGPL, missing licenses.
- As for your question: Fedora init scripts just should be 755, see
http://fedoraproject.org/wiki/Packaging/SysVInitScript.
- Since you want to retain %clean, you can remove the ifdef's around it. %clean
is not required, but certainly allowed.
- The %description should expand on the summary. It's now really short, a few
sentences about this being part of an ERP application and perhaps an URL would
be nice :)

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