Bug 817303 - Review Request: php-symfony2-Yaml - Symfony2 Yaml Component
Summary: Review Request: php-symfony2-Yaml - Symfony2 Yaml Component
Keywords:
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:
Depends On: php-channel-symfony2
Blocks: 823046 823060 823065 823066 837669
TreeView+ depends on / blocked
 
Reported: 2012-04-28 19:12 UTC by Shawn Iwinski
Modified: 2012-07-04 16:25 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-06-26 21:24:12 UTC
Type: ---
Embargoed:
fedora: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
review.txt (7.39 KB, text/plain)
2012-05-28 07:40 UTC, Remi Collet
no flags Details

Description Shawn Iwinski 2012-04-28 19:12:48 UTC
Spec URL: http://people.redhat.com/siwinski/rpmbuild/SPECS/php-symfony2-Yaml.spec

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

Description: 
The Symfony2 YAML Component parses YAML strings to convert them to PHP arrays.
It is also able to convert PHP arrays to YAML strings.

YAML, YAML Ain't Markup Language, is a human friendly data serialization
standard for all programming languages. YAML is a great format for your
configuration files. YAML files are as expressive as XML files and as readable
as INI files.

The Symfony2 YAML Component implements the YAML 1.2 version of the
specification.

Comment 3 Shawn Iwinski 2012-05-20 18:16:57 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-date, php-iconv, php-json, php-mbstring, php-pcre
- Removed %defattr from %files section

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

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

Comment 5 Remi Collet 2012-05-28 07:39:50 UTC
About
   [ -f package2.xml ] || mv package.xml package2.xml
   mv package2.xml %{pear_name}-%{version}/%{name}.xml

This is a old hack (yes it works) to ensure than package v2 is used.

I often use

   # package.xml is version 2.0
   mv package.xml %{pear_name}-%{version}/%{name}.xml

This is not an issue, but make the spec simpler to read.


About %clean
As you have clean EL-5 stuff (Buildroot, %defattr) you should also remove the %clean section and the rm -rf %{buildroot} in %install

I will prefer to move documentation to correct location by fixing the package.xml (temporary hack waiting for upstream fix, see bug #823041)

In fact you don't need to requires (nor buildrequires) php-pear(PEAR), the PEAR classes are not used, only the installer. But the Guildelines says you must.. so keep it (I will probably propose a minor update to the PHP Guidelines)

> BuildRequires:    php-pear >= 1:1.4.9-1.2
> BuildRequires:    php-pear(PEAR)

The first could be removed (no more older version, even in EPEL) and is a duplicate of the second.


Please confirm your fas account (as we have 6 packages approved + this one near to be, I will soon sponsor you).

Comment 6 Remi Collet 2012-05-28 07:40:24 UTC
Created attachment 587175 [details]
review.txt

Generated by fedora-review 0.1.3

Comment 7 Shawn Iwinski 2012-05-30 17:18:26 UTC
I have questions concerning EPEL (specifically because I would like the symfony2 packages included in EPEL 6).  According to http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#EL6 there are no differences from Fedora packaging guidelines, but rpmlint reports errors.  Are these "errors" considered warnings for EPEL and ignored during review?
NOTE: I removed the "rm -rf" from the %install section and completely removed the %clean section before running the following commands.

> uname -a
Linux siwinski.csb 2.6.32-220.13.1.el6.x86_64 #1 SMP Thu Mar 29 11:46:40 EDT 2012 x86_64 x86_64 x86_64 GNU/Linux
> cat /etc/redhat-release 
Red Hat Enterprise Linux Workstation release 6.2 (Santiago)
> rpmlint --version
rpmlint version 0.94 Copyright (C) 1999-2007 Frederic Lepied, Mandriva
> rpmbuild --version
RPM version 4.8.0
> rpmlint php-symfony2-Yaml.spec
php-symfony2-Yaml.spec:92: E: files-attr-not-set
php-symfony2-Yaml.spec:93: E: files-attr-not-set
php-symfony2-Yaml.spec:94: E: files-attr-not-set
php-symfony2-Yaml.spec:95: E: files-attr-not-set
php-symfony2-Yaml.spec:96: E: files-attr-not-set
php-symfony2-Yaml.spec:97: E: files-attr-not-set
php-symfony2-Yaml.spec:98: E: files-attr-not-set
php-symfony2-Yaml.spec: W: no-cleaning-of-buildroot %clean
php-symfony2-Yaml.spec: W: no-buildroot-tag
php-symfony2-Yaml.spec: W: no-%clean-section
0 packages and 1 specfiles checked; 7 errors, 3 warnings.
> rpmbuild -ba php-symfony2-Yaml.spec
[...]
Wrote: /home/siwinski/rpmbuild/SRPMS/php-symfony2-Yaml-2.0.14-3.el6.src.rpm
Wrote: /home/siwinski/rpmbuild/RPMS/noarch/php-symfony2-Yaml-2.0.14-3.el6.noarch.rpm
[...]
> rpmlint /home/siwinski/rpmbuild/SRPMS/php-symfony2-Yaml-2.0.14-3.el6.src.rpm
php-symfony2-Yaml.src: W: spelling-error Summary(en_US) Symfony -> Symphony
php-symfony2-Yaml.src: W: spelling-error %description -l en_US Symfony -> Symphony
php-symfony2-Yaml.src:92: E: files-attr-not-set
php-symfony2-Yaml.src:93: E: files-attr-not-set
php-symfony2-Yaml.src:94: E: files-attr-not-set
php-symfony2-Yaml.src:95: E: files-attr-not-set
php-symfony2-Yaml.src:96: E: files-attr-not-set
php-symfony2-Yaml.src:97: E: files-attr-not-set
php-symfony2-Yaml.src:98: E: files-attr-not-set
php-symfony2-Yaml.src: W: no-cleaning-of-buildroot %clean
php-symfony2-Yaml.src: W: no-buildroot-tag
php-symfony2-Yaml.src: W: no-%clean-section
1 packages and 0 specfiles checked; 7 errors, 5 warnings.
> rpmlint /home/siwinski/rpmbuild/RPMS/noarch/php-symfony2-Yaml-2.0.14-3.el6.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 8 Shawn Iwinski 2012-05-30 17:55:17 UTC
(In reply to comment #5)
> About
>    [ -f package2.xml ] || mv package.xml package2.xml
>    mv package2.xml %{pear_name}-%{version}/%{name}.xml
> 
> This is a old hack (yes it works) to ensure than package v2 is used.
> 
> I often use
> 
>    # package.xml is version 2.0
>    mv package.xml %{pear_name}-%{version}/%{name}.xml
> 
> This is not an issue, but make the spec simpler to read.

I will make this change along with the review issue changes.  I like what you use much better than the old hack (I was just copying what I thought was the standard).


> About %clean
> As you have clean EL-5 stuff (Buildroot, %defattr) you should also remove
> the %clean section and the rm -rf %{buildroot} in %install

Will do


> I will prefer to move documentation to correct location by fixing the
> package.xml (temporary hack waiting for upstream fix, see bug #823041)

Will do


> In fact you don't need to requires (nor buildrequires) php-pear(PEAR), the
> PEAR classes are not used, only the installer. But the Guildelines says you
> must.. so keep it (I will probably propose a minor update to the PHP
> Guidelines)
> 
> > BuildRequires:    php-pear >= 1:1.4.9-1.2
> > BuildRequires:    php-pear(PEAR)
> 
> The first could be removed (no more older version, even in EPEL) and is a
> duplicate of the second.

I will remove "BuildRequires: php-pear >= 1:1.4.9-1.2" (I was just copying what I thought was the standard).  As you mentioned, I will keep "BuildRequires: php-pear(PEAR)" and "Requires: php-pear(PEAR)" for now per the guidelines.


> Please confirm your fas account (as we have 6 packages approved + this one
> near to be, I will soon sponsor you).

FAS account: siwinski


> we have 6 packages approved

All of my php-symfony2-* packages used the same template so I will need to update and repost them with these changes as well.

Comment 9 Remi Collet 2012-05-30 18:00:29 UTC
> Are these "errors" considered warnings for EPEL and ignored during review?

This are only false-positive because rpmlint version in RHEL-6 is outdated (0.94 while fedora have 1.4.6)

So you can really ignore them.

Comment 10 Shawn Iwinski 2012-05-31 16:58:02 UTC
Updated to upstream version 2.0.15 & updates per comment #5 and comment #6

- 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-Yaml.spec

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

Comment 11 Remi Collet 2012-06-01 15:31:23 UTC
-Version:          2.0.14
-Release:          3%{?dist}
+Version:          2.0.15
+Release:          1%{?dist}

OK

-BuildRequires:    php-pear >= 1:1.4.9-1.2

OK

-[ -f package2.xml ] || mv package.xml package2.xml
-mv package2.xml %{pear_name}-%{version}/%{name}.xml
-cd %{pear_name}-%{version}
+# package.xml is version 2.0
+mv package.xml %{pear_name}-%{version}/%{name}.xml

OK

-# Move documentation to correct location
-# NOTE: Remove this when upstream updates their package.xml with role="doc"
-#       for these files
-mkdir -p $RPM_BUILD_ROOT%{pear_docdir}/Symfony/Component/%{pear_name}
-mv -f \
-    $RPM_BUILD_ROOT%{pear_phpdir}/Symfony/Component/%{pear_name}/composer.json \
-    $RPM_BUILD_ROOT%{pear_phpdir}/Symfony/Component/%{pear_name}/LICENSE \
-    $RPM_BUILD_ROOT%{pear_phpdir}/Symfony/Component/%{pear_name}/README.md \
-    $RPM_BUILD_ROOT%{pear_docdir}/Symfony/Component/%{pear_name}
-

Fixed by upstreaml, great !

-%clean
-[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT

OK


All problem fixed.

Package APPROVED.

Comment 12 Remi Collet 2012-06-06 16:16:48 UTC
As you are now in the packager group, you can now open the SCN request and proceed to the package import.

Comment 13 Shawn Iwinski 2012-06-07 21:44:09 UTC
New Package SCM Request
=======================
Package Name: php-symfony2-Yaml
Short Description: Symfony2 Yaml Component
Owners: siwinski
Branches: f16 f17 el6
InitialCC:

Comment 14 Gwyn Ciesla 2012-06-08 12:39:00 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2012-06-09 19:01:39 UTC
php-symfony2-Yaml-2.0.15-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/php-symfony2-Yaml-2.0.15-1.fc17

Comment 16 Fedora Update System 2012-06-09 19:01:50 UTC
php-symfony2-Yaml-2.0.15-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/php-symfony2-Yaml-2.0.15-1.el6

Comment 17 Fedora Update System 2012-06-09 19:01:59 UTC
php-symfony2-Yaml-2.0.15-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/php-symfony2-Yaml-2.0.15-1.fc16

Comment 18 Fedora Update System 2012-06-10 01:28:40 UTC
php-symfony2-Yaml-2.0.15-1.fc17 has been pushed to the Fedora 17 testing repository.

Comment 19 Fedora Update System 2012-06-26 21:24:12 UTC
php-symfony2-Yaml-2.0.15-1.fc16 has been pushed to the Fedora 16 stable repository.

Comment 20 Fedora Update System 2012-06-26 21:31:03 UTC
php-symfony2-Yaml-2.0.15-1.fc17 has been pushed to the Fedora 17 stable repository.

Comment 21 Fedora Update System 2012-06-28 16:05:39 UTC
php-symfony2-Yaml-2.0.15-1.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.