[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

Matthias Saou <matthias at rpmforge.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matthias at rpmforge.net

--- Comment #62 from Matthias Saou <matthias at rpmforge.net> 2011-08-13 09:09:40 EDT ---
Here are a few minor nitpicks about the 6.0.2-6 spec files :
 * Big spaces vs. tabs indentation mess, which denotes a general lack of
attention to detail.
 * Included Patch1000 isn't applied. No indication as of why.
 * No %changelog entry for 6.0.2-6.
 * Maybe the first line should read something like "simplified" and not
"crippled".
 * Some non-relevant leftovers which should be taken care of, such as "perhaps
the matplotlib could yield for pytz, in Mdv >=2009.0", "Suggests:
postgresql-server >= 8.2", "I don't understand why this is needed at this
stage" and "Hope that the upstream one will do".
 * Lack of consistency in %description URLs (trailing "/").
 * Lack of consistency in section separation (why 2 empty lines between %prep
and %patch?).
 * My, oh my! I hadn't seen that "urban legend"-ish line for a long time : "[
-n "%{buildroot}" -a "%{buildroot}" != / ] && rm -rf %{buildroot}". Remove the
check, just rm : The buildroot can't ever be "/"... and it's in %clean already
anyway.
 * The few explicit "/" after %{buildroot} aren't needed (again, consistency
issue), as in %{buildroot}/%{_bindir} which should be %{buildroot}%{_bindir}.
 * Lots of consistency problems in file modes. The worst being server-check.sh
installed with mode 744 in %install but being included with mode 755 in %files
: Very confusing.
 * Consistency problems in scriplets. For instance once "|| :" is used, but
then "exit 0" is also used. Once $1 is quoted, once it's not...
 * Consistency problems in %files : openerp-server.* (no number) vs.
openerp_serverrc.5* (number).

Overall, the spec file could be much cleaner, making it much easier to read,
understand and review.

Blocker : The /var/run/ content needs to be handled differently. See
http://fedoraproject.org/wiki/Packaging:Tmpfiles.d to learn how.

Doubt that I have : Do the main configuration directory and the main
configuration file really need to belong and be writeable by the openerp user?
If the configuration can be saved at run time, then yes, but otherwise if it's
only meant to be modified by an administrator it would be best avoided : If
some vulnerability in the daemon process allows to dump content to any file, it
would be trivial to overwrite the configuration, then possibly do something
very nasty upon the next restart.

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