Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-Assetic.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-Assetic-1.1.0-0.1.alpha3.fc18.src.rpm Description: Assetic is an asset management framework for PHP. Fedora Account System Username: siwinski
I know "%{_datadir}/php" is only supposed to contain classes, but how should we handle this package's 'functions.php' file?
I don't see any reason why we couldn't add "functions" files in /usr/share/php. This make sense to have this in the include path. Mostly, we don't want to see doc, or test (file which don't need to be in the include path) in this dir. So this package seems ok (for layout). AFAIK, you cannot use 2 requires on the same dependency (rpm/yum limit) So, instead of Requires: foo >= x Requires: foo < y You have to used Requires: foo >= x Conflicts: foo >= y
(In reply to comment #2) > I don't see any reason why we couldn't add "functions" files in > /usr/share/php. > > This make sense to have this in the include path. > > Mostly, we don't want to see doc, or test (file which don't need to be in > the include path) in this dir. > > So this package seems ok (for layout). OK. Thanks. > AFAIK, you cannot use 2 requires on the same dependency (rpm/yum limit) > So, instead of > Requires: foo >= x > Requires: foo < y > You have to used > Requires: foo >= x > Conflicts: foo >= y I have not seen any issues with 2 requires on the same dependency on my Fedora 18 or RHEL 6 machines with either rpm or yum. I cannot find any supporting documentation though. This may also help with repoquery queries. Mind if I ping the devel list?
From my recent tests, you're right, it seems to work. Probably a very old bug which is fix now... >Mind if I ping the devel list? Of course, you can.
Created attachment 707336 [details] phpci.log phpci version 2.13.2.
Created attachment 707345 [details] 916405-php-Assetic-review.txt Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29 Buildroot used: fedora-rawhide-x86_64 Command line :/usr/bin/fedora-review -b 916405
No blocker, but [!]: Latest version is packaged. 1.0.4 is tagged. Why do you take a github snapshot, when upstream properly tag each release and provides a tarball ? From guidelines https://fedoraproject.org/wiki/Packaging:SourceURL#Github "If the upstream does create tarballs you should use them as tarballs provide an easier trail for people auditing the packages." Ex : https://github.com/kriswallsmith/assetic/archive/v1.1.0-alpha4.tar.gz [!]: Requires correct, justified where necessary. Where do you find info about the twig / symfony min / max version ?
Spec changes: https://github.com/siwinski/rpms/commit/51dc9bc48668a4482d16b49e300c34cd2ecc99b8 SPEC URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-Assetic.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-Assetic-1.1.0-0.2.alpha4.fc18.src.rpm (In reply to comment #7) > No blocker, but > > [!]: Latest version is packaged. > 1.0.4 is tagged. Drupal 8 (future package I am working on) requires version 1.1.*. I just updated the package from v1.1.0-alpha3 to v1.1.0-alpha4. > Why do you take a github snapshot, when upstream properly tag each release > and provides a tarball ? > > From guidelines https://fedoraproject.org/wiki/Packaging:SourceURL#Github > "If the upstream does create tarballs you should use them as tarballs > provide an easier trail for people auditing the packages." > > Ex : https://github.com/kriswallsmith/assetic/archive/v1.1.0-alpha4.tar.gz From the same guidelines: "For a number of reasons (immutability, availability, uniqueness), you must use the full commit revision hash when referring to the sources. The full 40-character hash can be copied from the github web interface at https://github.com/$OWNER/$PROJECT/tags or by cloning the repository and using git rev-parse $TAG" I read that as we must use the full commit hash for sources rather than the tag. > [!]: Requires correct, justified where necessary. > Where do you find info about the twig / symfony min / max version ? From the composer.json file -- https://github.com/kriswallsmith/assetic/blob/v1.1.0-alpha4/composer.json "symfony/process": ">=2.1.0,<2.3-dev" "twig/twig": ">=1.6.0,<2.0"
Well... this Guidelines is a bit ambiguous. As far as the version / release are correct, and Source0 available (one way or another), I don't see ant blockers. === APPROVED ===
THANKS for the review! New Package SCM Request ======================= Package Name: php-Assetic Short Description: Asset Management for PHP Owners: siwinski Branches: f18 el6 InitialCC:
Git done (by process-git-requests).
php-Assetic-1.1.0-0.2.alpha4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-Assetic-1.1.0-0.2.alpha4.el6
php-Assetic-1.1.0-0.2.alpha4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/php-Assetic-1.1.0-0.2.alpha4.fc18
php-Assetic-1.1.0-0.2.alpha4.el6 has been pushed to the Fedora EPEL 6 testing repository.
php-Assetic-1.1.0-0.2.alpha4.fc18 has been pushed to the Fedora 18 stable repository.
php-Assetic-1.1.0-0.2.alpha4.el6 has been pushed to the Fedora EPEL 6 stable repository.