Bug 1047111 (php-doctrine-orm)

Created attachment 842931 [details]
php-doctrine-DoctrineORM.repoquery.txt

repoquery of pkgs requiring php-doctrine-DoctrineORM

Created attachment 845255 [details]
phpci.log

phpcompatinfo version 2.26.0.

Created attachment 845256 [details]
phpci.log

phpcompatinfo version 2.26.0.

Created attachment 845257 [details]
review.txt

Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30
Command line :/usr/bin/fedora-review -b 1047111

[!] Package installs properly.
  Ok, with all the new doctrine stack

[!]: Requires correct, justified where necessary.
  From composer.json
        "symfony/console": "~2.0"
        "symfony/yaml": "~2.1",
  So you can't use %{symfony_min_ver} for both

[!]: Dist tag is present (not strictly required in GL).

Cosmetic:
  => sed xxx foo > bar (instead of cat)
  => install -pm 755 ...


Please review %description to match changes in php-doctrine-cache (no sub-packages)

About /usr/bin/doctrine.

Instead of creating a PSR-0 autoloader, I think it will be cleaner to use Doctrine one (as in the doctrine-dbal command).

   require_once '/usr/share/php/Doctrine/Common/ClassLoader.php';
   $classLoader = new \Doctrine\Common\ClassLoader('Doctrine');
   $classLoader->register();
   $classLoader = new \Doctrine\Common\ClassLoader('Symfony');
   $classLoader->register();


First test for upgrade path seems ok :)

(In reply to Remi Collet from comment #5)
> [!]: Requires correct, justified where necessary.
>   From composer.json
>         "symfony/console": "~2.0"
>         "symfony/yaml": "~2.1",
>   So you can't use %{symfony_min_ver} for both

All Symfony components require each other's version so they should all be in-sync version-wise and it simplifies this spec to just take the ceiling of the minimum version.

> [!]: Dist tag is present (not strictly required in GL).

Fixed

> Cosmetic:
>   => sed xxx foo > bar (instead of cat)
>   => install -pm 755 ...

Updated

> Please review %description to match changes in php-doctrine-cache (no
> sub-packages)

Updated

> About /usr/bin/doctrine.
> 
> Instead of creating a PSR-0 autoloader, I think it will be cleaner to use
> Doctrine one (as in the doctrine-dbal command).
> 
>    require_once '/usr/share/php/Doctrine/Common/ClassLoader.php';
>    $classLoader = new \Doctrine\Common\ClassLoader('Doctrine');
>    $classLoader->register();
>    $classLoader = new \Doctrine\Common\ClassLoader('Symfony');
>    $classLoader->register();

Updated.  Much nicer.  THANKS!



Updates:
- Conditional %%{?dist}
- Bin script patch instead of inline update and use Doctrine Common classloader
- Updated optional cache information in %%description
- Removed empty file
- Removed unnecessary executable bit

Diff: https://github.com/siwinski/rpms/compare/9872cb2f77b8146ae4709ca1fd7f7e83c500910f...66d248d6ca984af2e696a993adcaef0d1efc81c6



Spec URL: https://raw.github.com/siwinski/rpms/66d248d6ca984af2e696a993adcaef0d1efc81c6/php-doctrine-orm.spec

SRPM URL: http://siwinski.fedorapeople.org/SRPMS/php-doctrine-orm-2.4.1-2.fc20.src.rpm

(In reply to Shawn Iwinski from comment #6)

> All Symfony components require each other's version so they should all be
> in-sync version-wise and it simplifies this spec to just take the ceiling of
> the minimum version.

Yes, I forget how this upstream is unable to properly manage their release.

[x]: Requires correct, justified where necessary.

[x]: Dist tag is present (not strictly required in GL).

/usr/bin/doctrine is ok.

[x] Package installs properly.
all the stack is now approved, so OK.


I think you can probably import the new packages in rawhide to allow everyone to test them (announce on devel@).

Remember to remove the old packages "only" in rawhide.

For other branches a longer "testing" period will be useful to allow more test and avoid late discovered issue (we have learn from symfony). Especially in EPEL.


== APPROVED ==

THANKS for the review!


New Package SCM Request
=======================
Package Name: php-doctrine-orm
Short Description: Doctrine Object-Relational-Mapper (ORM)
Owners: siwinski
Branches: f19 f20 el6
InitialCC:

Git done (by process-git-requests).