[Bug 914988] New: Review Request: php-SymfonyCmfRouting - Extends the Symfony2 routing component for dynamic routes and chaining several routers
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Bug ID: 914988 Summary: Review Request: php-SymfonyCmfRouting - Extends the Symfony2 routing component for dynamic routes and chaining several routers 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-SymfonyCmfRouting.spec
SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-SymfonyCmfRouting-1.0.0-...
Description: The Symfony CMF Routing component library extends the Symfony2 core routing component. Even though it has Symfony in its name, it does not need the full Symfony2 framework and can be used in standalone projects. For integration with Symfony we provide RoutingExtraBundle.
At the core of the Symfony CMF Routing component is the ChainRouter, that is used instead of the Symfony2's default routing system. The ChainRouter can chain several RouterInterface implementations, one after the other, to determine what should handle each request. The default Symfony2 router can be added to this chain, so the standard routing mechanism can still be used.
Additionally, this component is meant to provide useful implementations of the routing interfaces. Currently, it provides the DynamicRouter, which uses a RequestMatcherInterface to dynamically load Routes, and can apply RouteEnhancerInterface strategies in order to manipulate them. The provided NestedMatcher can dynamically retrieve Symfony2 Route objects from a RouteProviderInterface. This interfaces abstracts a collection of Routes, that can be stored in a database, like Doctrine PHPCR-ODM or Doctrine ORM. The DynamicRouter also uses a UrlGenerator instance to generate Routes and an implementation is provided under ProviderBasedGenerator that can generate routes loaded from a RouteProviderInterface instance, and the ContentAwareGenerator on top of it to determine the route object from a content object.
Fedora Account System Username: siwinski
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@famillecollet.com Alias| |SymfonyCmfRouting
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #1 from Remi Collet fedora@famillecollet.com --- Same comment for Requires: foo >= x Requires: foo < y Should use Requires: foo >= x Conflicts: foo >= y
Can you please move test out of include_path ?
Notice : probably you can even not ship the test units. (they are usefull during build, but I don't think our users will need them)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #2 from Shawn Iwinski shawn.iwinski@gmail.com --- (In reply to comment #1)
Same comment for Requires: foo >= x Requires: foo < y Should use Requires: foo >= x Conflicts: foo >= y
Can you please move test out of include_path ?
The reason why I kept the tests in the include path is because: 1) they were just classes that could be loaded with a common autoloader 2) they were only installed with the *-tests sub-package
Also, I ran into an issue with a package (Symfony's TwigBridge to be exact) that required test classes from another package (Symfony's Form) to be in the include path.
Notice : probably you can even not ship the test units. (they are usefull during build, but I don't think our users will need them)
OK. It was a little bit of a pain to sub-package tests. I will just not package them (but still run them in %check).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #3 from Shawn Iwinski shawn.iwinski@gmail.com --- Removed tests sub-package. Providing release in case we find out that we can use 2 requires on the same dependency.
Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-SymfonyCmfRouting.spec
SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-SymfonyCmfRouting-1.0.0-...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fedora@famillecollet.com
--- Comment #4 from Remi Collet fedora@famillecollet.com --- Created attachment 707414 --> https://bugzilla.redhat.com/attachment.cgi?id=707414&action=edit phpci.log
phpci version 2.13.2.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #5 from Remi Collet fedora@famillecollet.com --- Created attachment 707415 --> https://bugzilla.redhat.com/attachment.cgi?id=707415&action=edit 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 914988
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
--- Comment #6 from Remi Collet fedora@famillecollet.com --- [!]: Rpmlint is run on all rpms the build produces. php-SymfonyCmfRouting.noarch: E: summary-too-long C Extends the Symfony2 routing component for dynamic routes and chaining several routers
[!]: Package is named according to the Package Naming Guidelines. same comment about github snapshot. https://github.com/symfony-cmf/Routing/archive/1.0.0-alpha4.tar.gz Release: 0.2.alpha4%{dist} is enough (date + rev don't make sense here)
[!]: %check is present and all tests pass. Local build (with symfony installed) Fatal error: Interface 'Psr\Log\LoggerInterface' not found Mock build 3 skipped tests
[!]: All build dependencies are listed in BuildRequires, except for any that BuildRequires: php-PsrLog BuildRequires: php-pear(pear.symfony.com/Routing) BuildRequires: php-pear(pear.symfony.com/HttpKernel)
[!]: Requires correct, justified where necessary. According to recent discussion on -devel 2 requires seems ok.
Probably need (else please justify) Requires: php-PsrLog
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #7 from Shawn Iwinski shawn.iwinski@gmail.com --- Spec changes: https://github.com/siwinski/rpms/commit/82aa35baf46e661917cf1b01c44c5bc84883...
SPEC URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-SymfonyCmfRouting.spec
SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-SymfonyCmfRouting-1.0.0-...
(In reply to comment #6)
[!]: Rpmlint is run on all rpms the build produces. php-SymfonyCmfRouting.noarch: E: summary-too-long C Extends the Symfony2 routing component for dynamic routes and chaining several routers
Fixed
[!]: Package is named according to the Package Naming Guidelines. same comment about github snapshot. https://github.com/symfony-cmf/Routing/archive/1.0.0-alpha4.tar.gz Release: 0.2.alpha4%{dist} is enough (date + rev don't make sense here)
I agree I should have used just "0.2.alpha4%{dist}" when packaging the exact tagged version. However, I just updated the package to the latest snapshot because of Symfony 2.2 fixes -- additional commits beyond the 1.0.0-alpha4 version.
[!]: All build dependencies are listed in BuildRequires, except for any that BuildRequires: php-PsrLog BuildRequires: php-pear(pear.symfony.com/Routing) BuildRequires: php-pear(pear.symfony.com/HttpKernel)
Fixed Symfony build requires, however, see note below regarding PSR Log package.
[!]: %check is present and all tests pass. Local build (with symfony installed) Fatal error: Interface 'Psr\Log\LoggerInterface' not found Mock build 3 skipped tests
[!]: Requires correct, justified where necessary. According to recent discussion on -devel 2 requires seems ok.
Probably need (else please justify) Requires: php-PsrLog
I believe you are seeing this error because of your local HttpKernel 2.2 package [1] not requiring it's new PSR Log dependency [2]. Koji scratch build (which currently has Symfony 2.1 versions) build successfully [3].
[1] I'm assuming https://github.com/remicollet/remirepo/blob/master/php/symfony2/php-symfony2...
[2] https://github.com/symfony/HttpKernel/blob/v2.2.0/composer.json#L22
[3] http://koji.fedoraproject.org/koji/taskinfo?taskID=5100062
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
--- Comment #8 from Remi Collet fedora@famillecollet.com --- Ok, for the explanation about PsrLog, my mistake.
So, when you will update Symfony to 2.2 you will have to update to phpunit command to add %{_datadir}/php in the include_path (for PsrLog).
As scratch build is ok with current Symfony version, I don't see any blocker.
=== APPROVED ===
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #9 from Shawn Iwinski shawn.iwinski@gmail.com --- THANKS for the review!
New Package SCM Request ======================= Package Name: php-SymfonyCmfRouting Short Description: Extends the Symfony2 routing component Owners: siwinski Branches: f18 el6 InitialCC:
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #10 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-SymfonyCmfRouting-1.0.0-0.3.alph...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/php-SymfonyCmfRouting-1.0.0-0.3.alph...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.el6 has been pushed to the Fedora EPEL 6 testing repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2013-03-21 20:43:10
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.fc18 has been pushed to the Fedora 18 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914988
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.el6 has been pushed to the Fedora EPEL 6 stable repository.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla