[Bug 680205] Review Request: allegro5 - Allegro 5 is a game programming library.

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

Hans de Goede <hdegoede at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hdegoede at redhat.com

--- Comment #2 from Hans de Goede <hdegoede at redhat.com> 2011-02-24 14:46:17 EST ---
(In reply to comment #1)
> (Forgot to fill in the summary... :( Sorry about that. Fixed)

No problem, I'll review this and assuming all goes well eventually sponsor you
as discussed by email. Note I'm going on a business trip (leaving tomorrow,
back in a week), and I likely won't get around to any in depth review before
then.

Some initial comment based on a quick look at the spec file. First of all, this
looks pretty good for a first package!

Other remarks:
-Your spec file contents is ordered like this:
  -main package "tags" (key value pairs)
  -main package description
  -prep
  -build
  -install
  -clean
  -main package files
  -changelog
  -sub-package1 tags
  -sub-package1 desc
  -sub-package1 files
  -sub-package2 tags
  -sub-package2 desc
  -sub-package2 files
 This is rather unusual, the usual way to do this is:
  -main package "tags" (key value pairs)
  -main package description
  -sub-package1 tags
  -sub-package1 desc
  -sub-package2 tags
  -sub-package2 desc
  -prep
  -build
  -install
  -clean
  -main package files
  -sub-package1 files
  -sub-package2 files
  -changelog
 Also see for example:
 /etc/rpmdevtools/spectemplate-lib.spec
 (yum install rpmdevtools)

-Please split the long BuildRequires line to fit into 80
 char wide terminals, note you can do this like this:
 BuildRequires: foo bar
 BuildRequires: more and stuff

-You've created rather a lot of subpackages, this feels very Debian-esque
 For example I personally would:
 -put the man pages in the main -devel package
 -put most of the addons in the main package, some rules of thumb:
  -does it drag in extra dependencies, or just make the package
   slightly larger, if no extra deps put it in the main package
   (I think that this will apply to: addon-color, addon-dialog, addon-main,
    addon-memfile and addon-primitives)
  -will it likely be needed in most usage scenarios, if yes put it
   in the main package. I think this applies to addon-audio, addon-image
  addon-physfs otoh indeed belongs in a subpackage, not sure about
  addon-fonts and addon-ttf

-All (sub)packages which require other (sub)packages from the same
 package must do so by the full N-V-R (name version release), ie:
 Requires:       %{name} = %{version}-%{release}

-Drop all the Requires on libs, rpm will autogenerate these as soname  
 dependencies.

-Some of the -devel subpackage Requires on other -devel subpackages can like
 be dropped too, as rpm generates automatic cross devel dependencies based
 on pkgconfig files, see the actually generated dependencies in the
 build rpm ("rpm -qp --requires foo.rpm")

-try running rpmlint on the build rpms, and see if it finds anything useful:
 rpmlint *.src.rpm *.x86_64.rpm

-try running rpmlint on the installed package (does some other checks):
 rpmlint <installed-package-name>

Regards,

Hans

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