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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]