[Bug 517859] Review Request: vmpsd - vmpsd is a GPL implementation of Cisco Systems' VMPS

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


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

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




--- Comment #4 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-08-30 15:15:49 EDT ---
Some random notes:

* License
  - The license tag should be "GPL+", because no version information
    is given in the tarball (just putting GPLv2 license in the tarball
    does not specify the version of GPL: see below)
   
https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F

* URL
  - The following is more proper:
    http://vmps.sourceforge.net/

* BuildRequires
  - BR: gcc is redundant.
    https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

* Building source
  - Unless some specialy reason exists, compiling source codes (by "make",
    for example) must be done at %build stage and %install stage should
    do just installing files (by "make install", for example) and so on.
    (i.e. add "make %{?_smp_mflags}" after %configure, also see below)
    https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make

* Timestamp
  - Please consider to use
----------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
----------------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated from recent
    autotools.

* Creating scripts by cat in spec file
  - Please avoid this as much as possible, just create a text
    files beforehand, add these as %SOURCEx, and install
    the text files at %install (also see:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

* About scriptlets treatment
  ! Please read:
    https://fedoraproject.org/wiki/Packaging/SysVInitScript
    Some notes:
    - Some Requires(pre) or so is missing.
     
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets
    - Service should not be enabled by default:
     
https://fedoraproject.org/wiki/Packaging/SysVInitScript#.23_chkconfig:_line
    - Please use %_initddir
     
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem
    - Consier to add "condrestart" at %postun

* Macros
  - Please use macros properly.
    https://fedoraproject.org/wiki/Packaging/RPMMacros
    - Use %{_sysconfdir}, %{_bindir} and so on.
    - %{_datadir/man} should be %{_mandir}

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