[Bug 1715] Review request: icecast Streaming Media Server

[RPM Fusion Bugzilla]

http://bugzilla.rpmfusion.org/show_bug.cgi?id=1715


Richard <hobbes1069 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hobbes1069 at gmail.com




--- Comment #1 from Richard <hobbes1069 at gmail.com>  2011-04-25 22:47:15 ---
I'm a very new package maintainer so this isn't a full review but just my
thoughts...

1. I don't think you should have "Vendor:" specified.

2. I don't see anything in your BuildRequires for MP3 support or anything not
available in standard Fedora. If it doesn't have any BR for lame or other mp3
encoder and it can function without it (i.e., stream only OGG) then I don't see
any reason this couldn't be in the standard Fedora repos.

3. Why are you spec'ing a Prefix:? I'm not sure it does what you think it does
unless you're trying to build a relocatable package.

http://www.rpm.org/max-rpm-snapshot/s1-rpm-reloc-building-relocatable.html

Since you're using the same macro, %{_prefix} in your ./configure statement as
your assigning to "Prefix:" I don't think it's really doing much of anything.

4. It's still in the standard spec templates but "BuildRoot:" is no longer
needed and rpmbuild will ignore it.

5. As far as your ./configure line, have you tried using the "%configure"
macro? Did it not handle something properly?

6. I think you can condense both instances of:
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
to
rm -rf $RPM_BUILD_ROOT

Since it rpmbuild ignores your BuildRoot it should never be "/" and you should
never build a package as root so "/" shouldn't be affected.

Other thoughts:
If you really want to make sure you got the BuildRequires: right, have you
tried building your package with mock?

http://fedoraproject.org/wiki/Extras/MockTricks

That's all I can think of now without actually building your package. Hopefully
others will chime in.

Since this is your first package, I think this needs to block bug 30 as well
for "NEEDSPONSORS".


-- 
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.