[Bug 499137] Review Request: sipwitch - SIP telephony server for secure phone systems

[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=499137


Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp




--- Comment #1 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-05-06 14:08:17 EDT ---
>From very quick glance at your spec file:

- Fedora suggests that one line in %description must
  not have more than 79 characters
  (please check rpmlint warnings)
- Use macros consistently.
----------------------------------------------------
    31  Requires: %{name} = %{version}-%{release}
    36  Requires: sipwitch = %{version}-%{release}
----------------------------------------------------
- Would you explain why some of the subpackage does not
  have the dependency for main package?
- %{buildroot} is missing:
----------------------------------------------------
%{__rm} -f %{_libdir}/*.la
%{__rm} -f %{_libdir}/sipwitch/*.la
----------------------------------------------------
- Perhaps "INSTALL" is not needed for rpm document files.
- Files under %_mandir are automatically marked as %doc.
- The main package must not own the directory %_sbindir,
  %_bindir themselves. 
- For initscripts related convention, 
  * please follow
   
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem
   
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscript_packaging
  From rpmlint:
  * service should not be enabled by default (i.e. change
-----------------------------------------------------
# chkconfig: 2345 95 15
-----------------------------------------------------
    to
-----------------------------------------------------
# chkconfig: - 95 15
-----------------------------------------------------
  * init script should have "status" entry
    (please also check "$ rpmlint -I no-status-entry")
  * init script should put a lock file in
    %_localstatedir/lock/subsys .
- {_datadir}/snmp{,/mibs} is not owned by the main package
  but it installs some files under these directories.
  These directories are owned by net-snmp-libs, currently this
  package (sipwitch) does not seem to require net-snmp-libs.
  Would you check if sipwitch should require net-snmp-libs?
  (or examine why some files are installed under %_datadir/snmp?)
- Using %_libdir/python* is wrong (by the way this does not
  seem to work on 64 bit architecture). Please follow
  https://fedoraproject.org/wiki/Packaging/Python#System_Architecture
- Because of some reasons (one of the reasons is to avoid
  selinux AVC denial), we always install byte-compiled .py{o,c}
  files altogether (note that these .py{o,c} files are automatically
  created by /usr/lib/rpm/brp-python-bytecompile).
  So
  - these files must appear in %files list (using sipwitch.py*
    is easier)
  - creating/removing byte-compiled python files in scriptlets
    (i.e. at %post or so) should be removed.
- Usually configuration files should be marked as
  %config(noreplace)
  https://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files
- When only /sbin/ldconfig is called on a scriptlet, use -p
  option to avoid unneeded shell call, like:
---------------------------------------------------------
%postun -p /sbin/ldconfig
---------------------------------------------------------
  (however also please check initscripts scriptlets convention)

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