[Bug 181445] Review Request: php-shout

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

Summary: Review Request: php-shout


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=181445





------- Additional Comments From matthias at rpmforge.net  2006-03-30 12:08 EST -------
Further "nitpicks" :-)
- Keep consistent between tabs and spaces. Only the two top lines had tabs.
- For extdir and apiver, you need "failovers" for when PHP isn't available, for
instance when simply building a source rpm to send to a build system, or when
using mach which expands these outside of the buildroot first, then inside later.
- Having "Minor %define fixes" in the %changelog is wrong. Put %%define!
- You can save half the install lines by using "install -D" to create the dirs.
- I'd put the phpize calls in the %build section, and possibly remove the
--clean one (arguable, I guess)
- I find it easier to put files in %doc in alphabetical order, it's easier to
chack if they're all properly included.

Since it seems you are the upstream author :
- php_shout.c has some "implicit declaration of function 'php_info_print_table_*"
- /php_shout.c has some "'return' with no value, in function returning non-void"
...and a few assorted warnings you might like to look into (it always gives a
bad impression to see so many).

Let me know what you think of all these points. I'll attach a spec file patch.
I'll still have to do some testing of the actual module later on :-)

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.