Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Bug ID: 916405 Summary: Review Request: php-Assetic - Asset Management for PHP Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Reporter: shawn.iwinski@gmail.com
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...
Description: Assetic is an asset management framework for PHP.
Fedora Account System Username: siwinski
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@famillecollet.com Alias| |php-Assetic
--- Comment #1 from Shawn Iwinski shawn.iwinski@gmail.com --- I know "%{_datadir}/php" is only supposed to contain classes, but how should we handle this package's 'functions.php' file?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #2 from Remi Collet fedora@famillecollet.com --- 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #3 from Shawn Iwinski shawn.iwinski@gmail.com --- (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?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #4 from Remi Collet fedora@famillecollet.com ---
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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fedora@famillecollet.com
--- Comment #5 from Remi Collet fedora@famillecollet.com --- Created attachment 707336 --> https://bugzilla.redhat.com/attachment.cgi?id=707336&action=edit phpci.log
phpci version 2.13.2.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #6 from Remi Collet fedora@famillecollet.com --- Created attachment 707345 --> https://bugzilla.redhat.com/attachment.cgi?id=707345&action=edit 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
--- Comment #7 from Remi Collet fedora@famillecollet.com --- 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 ?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #8 from Shawn Iwinski shawn.iwinski@gmail.com --- Spec changes: https://github.com/siwinski/rpms/commit/51dc9bc48668a4482d16b49e300c34cd2ecc...
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...
(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"
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
--- Comment #9 from Remi Collet fedora@famillecollet.com --- 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 ===
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #10 from Shawn Iwinski shawn.iwinski@gmail.com --- THANKS for the review!
New Package SCM Request ======================= Package Name: php-Assetic Short Description: Asset Management for PHP Owners: siwinski Branches: f18 el6 InitialCC:
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #11 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- php-Assetic-1.1.0-0.2.alpha4.el6 has been pushed to the Fedora EPEL 6 testing repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2013-03-21 20:48:24
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- php-Assetic-1.1.0-0.2.alpha4.fc18 has been pushed to the Fedora 18 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=916405
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- php-Assetic-1.1.0-0.2.alpha4.el6 has been pushed to the Fedora EPEL 6 stable repository.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla