Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: php-pear Alias: php-pear https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226295 ------- Additional Comments From chris.stone@xxxxxxxxx 2007-02-05 14:45 EST ------- (In reply to comment #8) > Thanks for the review! You're welcome, but we are only about 1/2 way done ;-) > > Fixed in -4: > - upstream do not provide a permanent archive URL for each version of the .phar > installer, so there is no URL to provide (yes, I know this sucks). I'll add a > comment to this effect See comment #9, I need to know why using a .phar is required instead of using the .tgz > - bogus Group Thanks > - pear.conf marked noreplace; /etc/rpm/macros.* should never be noreplace (and > that should be documented by standard not per spec file) Thanks. Please add a comment, something like: # macros.* should be replaced on updates Remember, you are not the only one who reads the spec file, and should you get hit by a bus, the next php-pear maintainer will appreciate the extra documentation. > > Won't fix: > - passing -q to %setup has no effect when -T is also used Filed a bug against rpmlint (bug #227389). Thanks for pointing this out. > - providing an noop %build adds no value Yet, rpm has "unpredictable results if not added". You can email Ville Skytta if you need further clarification on this, he has real world examples where this has caused problems. Better to play safe than sorry. *ALL* pear packages contain empty %build sections, php-pear should not be an exception. Again, this is something that takes 5 seconds to add, and causes no harm and helps documentation in the spec for other users. Please add. > - no idea what the issue with $RPM_SOURCE_DIR is, this has been used in spec > files forever You should install source files using %{SOURCEx} notation, please change your for loop to something like this: install -pm 755 %{SOURCE10} $RPM_BUILD_ROOT%{_bindir}/pear install -pm 755 %{SOURCE11} $RPM_BUILD_ROOT%{_bindir}/pecl install -pm 755 %{SOURCE12} $RPM_BUILD_ROOT%{_bindir}/peardev In addition, your reference to macros.pear should use %{SOURCE13} > - the patches are applied using %{PATCHn} syntax My mistake. You are right in this case, pear packages are installed funky and patches have to be done this way. > - the license specified in every PEAR class file is indeed v3 not v2.02. There > is no accepted policy for the License tag in Fedora yet; there is no point > tweaking this on a whim until there is, rpmlint is not definitive on that front Okay, can you give me some kind of proof that _all_ pear classes are automatically licensed with PHP License 3.0? I know of many pear classes that use BSD for example. The upstream home page actually has a link to the license file which is a version 2.02 license. I simply cannot approve this until this matter is cleared up. The licenses **MUST** match upstream. If upstream's home page is pointing to the wrong license, then please attach to this bug report an e-mail from upstream saying their home page is incorrect. Please also remove the "The" and "v" in the License tag. Why not atleast make the licenses consistent to help people who run shell scripts and such parsing license tags? A license of "PHP License 3.0" is no different than "The PHP License v3.0", however the first case is consistent with all other packages licensed with a PHP license. > > Again, the upgrade to the latest upstream is not relevant to the review of the > current packaging and need not block the review process. Of course the > packaging may change with each new upstream releases, that is always going to be > true. Right, so what is the point in reviewing a package that is going to change significantly immediately after it is reviewed? It makes far greater sense to make the significant changes _before_ the review. -- 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-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review