[Bug 705043] Review Request: paco - a source code package organizer for Unix

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

Martin Gieseking <martin.gieseking at uos.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |martin.gieseking at uos.de

--- Comment #5 from Martin Gieseking <martin.gieseking at uos.de> 2011-07-17 07:23:08 EDT ---
Hi Veeti, here are some comments on your latest package:

- if you don't plan to maintain the package for EPEL < 6, you can drop all the
  buildroot stuff (BuildRoot field, %clean section, initial cleaning of the
  buildroot in %install)

- you should ask upstream to properly apply the GPL by adding the text given in 
  COPYING to the source files:

   <one line to give the program's name and a brief idea of what it does.>
   Copyright (C) <year>  <name of author>

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2 of the License, or
   (at your option) any later version.

   ...

  Without these information it's not clear what license is actually intended
  (GPL+, GPLv2, GPLv2+). Currently, it's actually GPL+ as relying on COPYING
  is not sufficient. Also see the information given here:
  http://fedoraproject.org/wiki/Licensing:Main

- The rpmlint warning shared-lib-calls-exit should be fixed upstream.

- I suggest to prefix the patch file names with the package name to avoid
  conflicts and to easily find the patches in the source directory:
  paco-fix-fsf-address.patch
  paco-fix-desktop-file.patch

- Giving the full icon path in the .desktop file is fine, but it's 
  recommended to simply use the basename of the icon file:
  http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files

- It's common practice to list all BuildRequires at the top part of the spec
  file, e.g. below BuildRoot or the PatchXXX lines -- even those of the
  subpackages. This allows to get an overview of the direct dependencies 
  easily.

- You should preserve the timestamp of file ChangeLog, e.g. with 
  iconv -f iso8859-1 -t utf-8 ChangeLog > ChangeLog.conv && \
  touch -r ChangeLog ChangeLog.conv && \
  mv -f ChangeLog.conv ChangeLog

- Directory %{_datadir}/paco/ only contains README, faq.txt, and pacorc.
  README is already packaged as %doc, and pacorc is installed in /etc.
  Thus, I recommend to drop %{_datadir}/paco/ completely. You can add faq.txt
  as %doc as well.

- Please be a bit more verbose in %files. This helps to avoid adding unwanted 
  files and gives a better overview of the package's content:

  %{_bindir}/ocap
  %{_bindir}/paco*
  %{_bindir}/rpm2paco
  %{_bindir}/superpaco
  %{_libdir}/libpaco-log.so.*
  %{_mandir}/man5/pacorc.5*
  %{_mandir}/man8/*.8*

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