Bug 823046 - Review Request: php-symfony2-DependencyInjection - Symfony2 DependencyInjection Component
Summary: Review Request: php-symfony2-DependencyInjection - Symfony2 DependencyInjecti...
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Remi Collet
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: php-channel-symfony2 817303 823042
Blocks: 823073
TreeView+ depends on / blocked
 
Reported: 2012-05-18 21:25 UTC by Shawn Iwinski
Modified: 2012-07-03 19:32 UTC (History)
3 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2012-07-02 22:28:35 UTC
fedora: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
php-symfony2-DependencyInjection-review.txt (5.97 KB, text/plain)
2012-06-06 17:11 UTC, Remi Collet
fedora: review?
Details

Description Shawn Iwinski 2012-05-18 21:25:40 UTC
Spec URL:
http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-DependencyInjection.spec

SRPM URL:
http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-DependencyInjection-2.0.14-1.fc16.src.rpm

Description:
The Dependency Injection component allows you to standardize and centralize the
way objects are constructed in your application.

For an introduction to Dependency Injection and service containers see
Service Container (http://symfony.com/doc/current/book/service_container.html).

Comment 1 Shawn Iwinski 2012-05-20 17:47:15 UTC
Updates per comments in bug 823043

- Removed BuildRoot
- Changed php require to php-common
- Added the following requires based on phpci results:
  php-ctype, php-dom, php-libxml, php-pcre, php-spl, php-simplexml
- Removed %defattr from %files section

SPEC URL:
http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-DependencyInjection.spec

SRPM URL:
http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-DependencyInjection-2.0.14-2.fc16.src.rpm

Comment 3 Shawn Iwinski 2012-05-31 17:09:38 UTC
Updated to upstream version 2.0.15 & updates per bug #817303

- Removed "BuildRequires: php-pear >= 1:1.4.9-1.2"
- Updated %prep section
- Removed cleaning buildroot from %install section
- Removed documentation move from %install section (fixed upstream)
- Removed %clean section
- Updated %doc in %files section

SPEC URL: http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-DependencyInjection.spec

SRPM URL: http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-DependencyInjection-2.0.15-1.fc16.src.rpm

Comment 4 Remi Collet 2012-06-06 17:11:24 UTC
Created attachment 589962 [details]
php-symfony2-DependencyInjection-review.txt

Generated by fedora-review 0.1.3

Comment 5 Remi Collet 2012-06-06 17:13:10 UTC
SHOULD: From package.xml, there are 2 optionnal deps.
php-pear(pear.symfony.com/Config)
php-pear(pear.symfony.com/Yaml)


It's a packager choice to make this (mandatory) Requires.

If you choose to add this requires, you can also remove
%dir %{pear_phpdir}/Symfony/Component
%dir %{pear_phpdir}/Symfony


No blocker.

=== APPROVED ===

Comment 6 Shawn Iwinski 2012-06-07 21:43:11 UTC
New Package SCM Request
=======================
Package Name: php-symfony2-DependencyInjection
Short Description: Symfony2 DependencyInjection Component
Owners: siwinski
Branches: f16 f17 el6
InitialCC:

Comment 7 Gwyn Ciesla 2012-06-08 12:42:55 UTC
Git done (by process-git-requests).

Comment 8 Remi Collet 2012-06-10 09:54:33 UTC
So for late error detection

DependencyInjection/Loader/schema/dic/services/services-1.0.xsd should be installed as role="php", not as role="doc" as this path is referred in the code

In DependencyInjection/Loader/XmlFileLoader.php

   $schemaLocations = array('http://symfony.com/schema/dic/services' 
   => str_replace('\\', '/', __DIR__.'/schema/dic/services/services-1.0.xsd'));


Like for "Locale" this should be reported upstream, and role="data" is probably a better solution.

Comment 9 Shawn Iwinski 2012-06-10 22:45:12 UTC
(In reply to comment #8)
> So for late error detection
> 
> DependencyInjection/Loader/schema/dic/services/services-1.0.xsd should be
> installed as role="php", not as role="doc" as this path is referred in the
> code
> 
> In DependencyInjection/Loader/XmlFileLoader.php
> 
>    $schemaLocations = array('http://symfony.com/schema/dic/services' 
>    => str_replace('\\', '/',
> __DIR__.'/schema/dic/services/services-1.0.xsd'));
> 
> 
> Like for "Locale" this should be reported upstream, and role="data" is
> probably a better solution.

Thanks for finding this Remi.  I will make sure to fix the issue before proceeding.

I will get upstream to fix the package.xml role for the *.xsd file.  It will take a code change to allow for role="data", but I will work with upstream to fix the issue.  If role="data" will take some time to implement, is it fine to revert to role="php" for now and plan on role="data" for a future enhancement?

I looked through all the other php-symfony2-* packages and it appears this same error exists in version 2.0.15 for the Routing, Translation, and Validator packages as well.  I will update those reviews with this information and work with upstream to fix.

Since these fixes upstream will require a new release (probably 2.0.16), may I just fix the package.xml in the spec file?

Comment 10 Remi Collet 2012-06-11 04:50:51 UTC
(In reply to comment #9)
> If role="data" will take some time to implement, is it
> fine to revert to role="php" for now and plan on role="data" for a future
> enhancement?

Yes.

> Since these fixes upstream will require a new release (probably 2.0.16), may
> I just fix the package.xml in the spec file?

Yes

Comment 12 Fedora Update System 2012-06-13 05:35:30 UTC
php-symfony2-DependencyInjection-2.0.15-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/php-symfony2-DependencyInjection-2.0.15-3.fc17

Comment 13 Fedora Update System 2012-06-13 05:35:39 UTC
php-symfony2-DependencyInjection-2.0.15-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/php-symfony2-DependencyInjection-2.0.15-3.fc16

Comment 14 Fedora Update System 2012-06-13 05:35:49 UTC
php-symfony2-DependencyInjection-2.0.15-3.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/php-symfony2-DependencyInjection-2.0.15-3.el6

Comment 15 Fedora Update System 2012-06-15 00:20:23 UTC
php-symfony2-DependencyInjection-2.0.15-3.fc17 has been pushed to the Fedora 17 testing repository.

Comment 16 Fedora Update System 2012-07-02 22:28:35 UTC
php-symfony2-DependencyInjection-2.0.15-3.fc16 has been pushed to the Fedora 16 stable repository.

Comment 17 Fedora Update System 2012-07-02 22:33:21 UTC
php-symfony2-DependencyInjection-2.0.15-3.fc17 has been pushed to the Fedora 17 stable repository.

Comment 18 Fedora Update System 2012-07-03 19:32:36 UTC
php-symfony2-DependencyInjection-2.0.15-3.el6 has been pushed to the Fedora EPEL 6 stable repository.


Note You need to log in before you can comment on or make changes to this bug.