[Bug 785619] Review Request: lutok - Lightweight C++ API library for Lua

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

--- Comment #2 from Michel Alexandre Salim <michel+fdr at sylvestre.me> 2012-02-02 08:42:26 EST ---
Some short notes, we'll do the full review once this is taken care of.

- license field should be just "License: BSD". see the short name column in
  http://fedoraproject.org/wiki/Licensing#Good_Licenses

- will you be packaging this for Fedora only, or also for RHEL? Unless you're
targeting RHEL version 5 or below, you don't need to declare the BuildRoot, and
you don't need to clean it at the beginning of %install:

  http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

- likewise, %clean section can be removed unless you're targeting RHEL 6 or
below.

- %setup defaults to "-n %{name}-%{version}". Unless you do something unusual
here you can just do %setup -q

  (to think about it, we don't have a Lua package naming guideline yet. Not
sure
   if this should be lutok, lua-lutok, or lua-tok. The Python precedent is that
   packages starting with "py" don't need to be prefixed with python-, so
   in lieu of an explicit guideline the name seems OK for now)

- the INSTALL file describes a 'make check' step. I take it without ATF the
tests don't do anything, but since they run just fine anyway, could you add a
check section under %install, as such:

%check
make check

that way once ATF is packaged you don't have to change this package much. Since
you're the author of ATF as well, do you want to submit it for inclusion as
well?

- you're installing documentation twice -- %doc list-of-files copy the files to
%{_defaultdocdir}/%{name}-%{version}/ and you also have files in
%{_defaultdocdir}/%{name}. The former is canonical, just remove the latter as
part of %install, or update the build script to make doc installation a
separate step

- consider packaging examples and HTML docs in a separate -doc subpackage
- devel subpackage should Require: %{name} = %{version}-%{release} (it should
match the main package)

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