[Bug 726080] Review Request: Xnee - X11 environment recorder

bugzilla at redhat.com bugzilla at redhat.com
Mon Oct 17 15:43:53 UTC 2011


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

--- Comment #10 from Mohamed El Morabity <pikachu.2014 at gmail.com> 2011-10-17 11:43:52 EDT ---
That's look better know.
Just some tips:

- don't hesitate to use line breaks to make your .spec easier to read,
especially in %install.

- don't forget to comment all "uncommon" tasks in your RPM, for example:
  1) why do you delete these files:
  rm -f %{buildroot}%{_libdir}/libtestcb.*
  rm -f %{buildroot}%{_datadir}/xnee/simple_bash.sh
  2) why the additional options to %configure
  Comments helps the reviewer once. But they'll help you maintaining your
package each time you'll update it.

- You can remove the "BuildRoot" line, as well as all occurrences of "rm -rf
%{buildroot}". The %clean section is obsolete also. See below for more:
   http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
   http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

- You'd better perform these lines in %prep, instead of %install:
  chmod a-x libxnee/include/libxnee/feedback.h   \
          libxnee/src/feedback.c               \
          libxnee/src/xnee_setget.c            \
          libxnee/src/xnee_range.c             \
          libxnee/include/libxnee/xnee.h       \
          cnee/include/parse.h                 \
          libxnee/include/libxnee/xnee_range.h

- The INSTALL file is useless, even for the devel package. Don't include it.

- Are you really sure that all the .h files delivered in the sources form a
consistent API for libxnee (see the devel package)? You'd better ping upstream
about this, or, at least, check which headers are included in others
distributions' packages (see Debian for example, on
http://packages.debian.org/).

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



More information about the package-review mailing list