Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-SymfonyCmfRouting.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-SymfonyCmfRouting-1.0.0-0.1.alpha4.20130121git92ee467.fc18.src.rpm 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
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)
(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).
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-0.2.alpha4.20130121git92ee467.fc18.src.rpm
Created attachment 707414 [details] phpci.log phpci version 2.13.2.
Created attachment 707415 [details] 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
[!]: 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
Spec changes: https://github.com/siwinski/rpms/commit/82aa35baf46e661917cf1b01c44c5bc84883fb98 SPEC URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-SymfonyCmfRouting.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.fc18.src.rpm (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-HttpKernel/php-symfony2-HttpKernel.spec [2] https://github.com/symfony/HttpKernel/blob/v2.2.0/composer.json#L22 [3] http://koji.fedoraproject.org/koji/taskinfo?taskID=5100062
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 ===
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:
Git done (by process-git-requests).
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.alpha4.20130306git4706313.el6
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.alpha4.20130306git4706313.fc18
php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.el6 has been pushed to the Fedora EPEL 6 testing repository.
php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.fc18 has been pushed to the Fedora 18 stable repository.
php-SymfonyCmfRouting-1.0.0-0.3.alpha4.20130306git4706313.el6 has been pushed to the Fedora EPEL 6 stable repository.