Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: php-symfony2-Form - Symfony2 Form Component
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Summary: Review Request: php-symfony2-Form - Symfony2 Form Component Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: shawn.iwinski@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
Spec URL: http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-Form.spec
SRPM URL: http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-Form-2.0.14-1....
Description: Symfony2 Form Component.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends on| |814994(php-channel-symfony2 | |), 823050, 823066, 823056
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@famillecollet.com
--- Comment #1 from Remi Collet fedora@famillecollet.com 2012-05-19 02:50:18 EDT --- Just a quick notes
- LICENSE and README file are installed in /usr/share/pear.
Probably a good idea to ask upstream, this files should be tagged as "doc" and so installed in /usr/share/doc/pear (and avoid to be duplicated in the package)
- Extension dependencies
# phpci print --recursive --report extension Symfony ------------------------------------------------------------------------------- PHP COMPAT INFO EXTENSION SUMMARY ------------------------------------------------------------------------------- EXTENSION PECL VERSION COUNT ------------------------------------------------------------------------------- Core 4.0.0 309 SPL 0.2 5.0.0 5 ctype 4.0.4 3 date 4.0.0 10 intl 1.1.0 5.2.4 12 pcre 4.0.0 13 session 4.0.0 3 standard 4.0.0 689 -------------------------------------------------------------------------------
So need to requires php-intl
- Other classes dependencies
From package.xml => ok
Requires: php-pear((%{pear_channel}/EventDispatcher) Requires: php-pear((%{pear_channel}/Validator) Requires: php-pear((%{pear_channel}/Locale)
And optionnaly (your decision) Requires: php-pear((%{pear_channel}/HttpFoundation)
- directory ownership
%dir %{pear_phpdir}/Symfony/Component %dir %{pear_phpdir}/Symfony
You don't need to own this directories which are already owned by required packages.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #2 from Shawn Iwinski shawn.iwinski@gmail.com --- (In reply to comment #1)
Just a quick notes
- LICENSE and README file are installed in /usr/share/pear.
Probably a good idea to ask upstream, this files should be tagged as "doc" and so installed in /usr/share/doc/pear (and avoid to be duplicated in the package)
I will request the change upstream. If they refuse or take a very long to make the change, is this a blocker? Is it acceptable to use a sed command in the spec file to modify the package.xml file to change the roles of those files to doc?
- Extension dependencies
# phpci print --recursive --report extension Symfony
-- PHP COMPAT INFO EXTENSION SUMMARY
-- EXTENSION PECL VERSION COUNT
-- Core 4.0.0 309 SPL 0.2 5.0.0 5 ctype 4.0.4 3 date 4.0.0 10 intl 1.1.0 5.2.4 12 pcre 4.0.0 13 session 4.0.0 3 standard 4.0.0 689
--
So need to requires php-intl
I'm getting different phpci results and do not see the intl extension listed:
$ uname -a Linux siwinski-fedora 3.3.2-6.fc16.x86_64 #1 SMP Sat Apr 21 12:43:20 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux $ phpci --version phpci version 2.3.0. $ phpci print --recursive --report extension Form-2.0.14 121 / 121 [+++++++++++++++++++++++++++++++++++++++++++++++++++++++++>] 100.00% BASE: /home/siwinski/rpmbuild/SOURCES/Form-2.0.14 ------------------------------------------------------------------------------- PHP COMPAT INFO EXTENSION SUMMARY ------------------------------------------------------------------------------- EXTENSION PECL VERSION COUNT ------------------------------------------------------------------------------- Core 4.0.0 309 SPL 0.2 5.0.0 5 ctype 4.0.4 3 date 4.0.0 10 pcre 4.0.0 13 session 4.0.0 3 standard 4.0.0 689 ------------------------------------------------------------------------------- A TOTAL OF 7 EXTENSION(S) WERE FOUND REQUIRED PHP 5.0.0 (MIN) ------------------------------------------------------------------------------- Time: 7 seconds, Memory: 20.75Mb -------------------------------------------------------------------------------
- Other classes dependencies
From package.xml => ok Requires: php-pear((%{pear_channel}/EventDispatcher) Requires: php-pear((%{pear_channel}/Validator) Requires: php-pear((%{pear_channel}/Locale)
And optionnaly (your decision) Requires: php-pear((%{pear_channel}/HttpFoundation)
I would prefer to let end-users decide if they want to use optional PEAR packages
- directory ownership
%dir %{pear_phpdir}/Symfony/Component %dir %{pear_phpdir}/Symfony
You don't need to own this directories which are already owned by required packages.
I was just keeping the spec files consistent. May I just comment out the those 2 lines and add a note as to why they are commented out, or do you want me to delete them?
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #3 from Shawn Iwinski shawn.iwinski@gmail.com --- 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-date, php-pcre, php-session, php-spl - Removed %defattr from %files section - Removed ownership for directories already owned by required packages
Update per https://bugzilla.redhat.com/show_bug.cgi?id=823041#c5
- Moved documentation to correct location
SPEC URL: http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-Form.spec
SRPM URL: http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-Form-2.0.14-3....
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(fedora@famillecol | |let.com)
--- Comment #4 from Shawn Iwinski shawn.iwinski@gmail.com --- (In reply to comment #2)
(In reply to comment #1)
- Extension dependencies
# phpci print --recursive --report extension Symfony
-- PHP COMPAT INFO EXTENSION SUMMARY
-- EXTENSION PECL VERSION COUNT
-- Core 4.0.0 309 SPL 0.2 5.0.0 5 ctype 4.0.4 3 date 4.0.0 10 intl 1.1.0 5.2.4 12 pcre 4.0.0 13 session 4.0.0 3 standard 4.0.0 689
--
So need to requires php-intl
I'm getting different phpci results and do not see the intl extension listed:
$ uname -a Linux siwinski-fedora 3.3.2-6.fc16.x86_64 #1 SMP Sat Apr 21 12:43:20 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux $ phpci --version phpci version 2.3.0. $ phpci print --recursive --report extension Form-2.0.14 121 / 121 [+++++++++++++++++++++++++++++++++++++++++++++++++++++++++>] 100.00% BASE: /home/siwinski/rpmbuild/SOURCES/Form-2.0.14
-- PHP COMPAT INFO EXTENSION SUMMARY
-- EXTENSION PECL VERSION COUNT
-- Core 4.0.0 309 SPL 0.2 5.0.0 5 ctype 4.0.4 3 date 4.0.0 10 pcre 4.0.0 13 session 4.0.0 3 standard 4.0.0 689
-- A TOTAL OF 7 EXTENSION(S) WERE FOUND REQUIRED PHP 5.0.0 (MIN)
-- Time: 7 seconds, Memory: 20.75Mb
--
Remi -- Any idea about the phpci difference with the intl extension?
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(fedora@famillecol | |let.com) |
--- Comment #5 from Remi Collet fedora@famillecollet.com ---
Remi -- Any idea about the phpci difference with the intl extension?
phpci only reports about "installed" extensions. This means : you need to install all extensions to have them detect : yes this is awfull... :(
Another solution is to create a /etc/pear/PHP_CompatInfo/phpcompatinfo.xml
From : file:///usr/share/doc/pear/PHP_CompatInfo/docs/phpci-book.html The XML configuration above corresponds to the default behaviour of the phpcli tool. reference Data dictionnary reference name. Defaults to PHP5 for all PHP4 and PHP5 components depending of your extensions loaded.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #6 from Remi Collet fedora@famillecollet.com --- FYI: I have add a feature request on phpci to have an new reference "ALL" to avoid such detection problem. https://github.com/llaville/php-compat-info/pull/36
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #7 from Shawn Iwinski shawn.iwinski@gmail.com --- Update per comment #1 and comment #5
- Added missing php-intl require
SPEC URL: http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-Form.spec
SRPM URL: http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-Form-2.0.14-4....
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #8 from Shawn Iwinski shawn.iwinski@gmail.com --- 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-Form.spec
SRPM URL: http://people.redhat.com/siwinski/rpmbuild/SRPMS/php-symfony2-Form-2.0.15-1....
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Whiteboard| |NotReady
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |823054
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |823075
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Whiteboard|NotReady |
--- Comment #9 from Shawn Iwinski shawn.iwinski@gmail.com --- Added optional require
- Added php-pear(%{pear_channel}/HttpFoundation) require
SPEC URL: http://people.redhat.com/~siwinski/rpmbuild/SPECS/php-symfony2-Form.spec
SRPM URL: http://people.redhat.com/~siwinski/rpmbuild/SRPMS/php-symfony2-Form-2.0.15-2...
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fedora@famillecollet.com Flags| |fedora-review?
--- Comment #10 from Remi Collet fedora@famillecollet.com --- Can you please check the role for Symfony/Component/Form/Resources/config/validation.xml
I can't find any "direct" use of this file in the code, but role="doc" seems strange...
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #11 from Shawn Iwinski shawn.iwinski@gmail.com --- (In reply to comment #10)
Can you please check the role for Symfony/Component/Form/Resources/config/validation.xml
I can't find any "direct" use of this file in the code, but role="doc" seems strange...
This appears to be the same type issue with the XSD files in some of the other php-symfony2-* packages although I cannot find any code in this package that references that file either. I could ask upstream if we're missing something, but perhaps I could just change it's role to "php" (like I did for those XSD files) without acknowledgment from upstream as it seems this version (2.0.15) may be the last of the 2.0.x series as they already have betas out for 2.1. Recommendation?
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #12 from Remi Collet fedora@famillecollet.com --- (In reply to comment #11)
Recommendation?
If needed and set as "doc", the package will not work. If fixed to "php", and not used, the package will work.
So 1/ fix the role, 2/ ask confirmation to upstream, as this need to be fixed in next 2.1
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #13 from Shawn Iwinski shawn.iwinski@gmail.com --- Update per comment #10 and comment #12
- Changed PEAR role of Symfony/Component/Form/Resources/config/validation.xml (Note: This has been fixed in upstream version 2.1.0 BETA1)
SPEC URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-symfony2-Form.spec
SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/RPMS/noarch/php-symfony2-Form-2.0....
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #14 from Remi Collet fedora@famillecollet.com --- Created attachment 595409 --> https://bugzilla.redhat.com/attachment.cgi?id=595409&action=edit php-symfony2-Form-review.txt
Generated by fedora-review 0.1.3
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #15 from Remi Collet fedora@famillecollet.com --- in Form-2.0.15/Symfony/Component/Form/Extension/DependencyInjection/DependencyInjectionExtension.php
use Symfony\Component\DependencyInjection\ContainerInterface;
so I recommend to add this in the optional requires, and see with upstream to add this in the package.xml
No blocker, so
==== APPROVED ====
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #16 from Shawn Iwinski shawn.iwinski@gmail.com --- New Package SCM Request ======================= Package Name: php-symfony2-Form Short Description: Symfony2 Form Component Owners: siwinski Branches: f16 f17 el6 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #17 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-symfony2-Form-2.0.15-4.el6
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/php-symfony2-Form-2.0.15-4.fc17
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/php-symfony2-Form-2.0.15-4.fc16
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #21 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.fc16 has been pushed to the Fedora 16 testing repository.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2012-07-12 14:54:15
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.fc17 has been pushed to the Fedora 17 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #23 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.fc16 has been pushed to the Fedora 16 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- php-symfony2-Form-2.0.15-4.el6 has been pushed to the Fedora EPEL 6 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=823071
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |859271
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823071
Bug 823071 depends on bug 859271, which changed state.
Bug 859271 Summary: Review Request: php-symfony2-OptionsResolver - Symfony2 OptionsResolver Component https://bugzilla.redhat.com/show_bug.cgi?id=859271
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |ERRATA
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla