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@xxxxxxxxxxxx 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. -- fedora-extras-list mailing list fedora-extras-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-extras-list
