Spec URL: https://raw.github.com/siwinski/rpms/58afbf34df09331c037ce42a9f83d089708ec0ac/php-doctrine-cache.spec SRPM URL: http://siwinski.fedorapeople.org/SRPMS/php-doctrine-cache-1.3.0-1.fc20.src.rpm Description: Cache component extracted from the Doctrine Common project. Fedora Account System Username: siwinski
Created attachment 844836 [details] phpci.log
Created attachment 844837 [details] review.txt
Must: [!]: Package does not generate any conflict. Conflicts: php-pear(pear.doctrine-project.org/DoctrineCommon) < 2.4 This is ok, buth worth a comment to explain this is a split-off DoctrineCommon [!]: Dist tag is present (not strictly required in GL). Please use %{?dist} Cosmetic: [!]: %check is present and all tests pass. Also remove tests/Doctrine/Tests/Common/Cache/CouchbaseCacheTest.php which fails when couchbase extension is installed About the sub-package, I don't know what is better, to say "If you wand memcached handler, you need to install: - php-doctrine-cache-memcached (will pull php-pecl-memcached) OR only - php-pecl-memcached Both are acceptable, first is perhaps more confusing for user. I think you will have an issue when pecl/riak or pecl/couchbase will come to fedora repo (already in my repo, but not ready for review, mostly because of bundled lib) as you will have to move the file from main package to a new sub-package ? (this could create upgrade issue for users) So perhaps it will be simpler to keep all the handlers in the main package. This stay your packager choice.
- Conditional %{?dist} - Removed sub-packages -- This does make more sense and is easier this way... I don't know why I originally sub-packaged - Skip all tests requiring a server to connect to Diff: https://github.com/siwinski/rpms/commit/8087060ca33da63e33848809b3215dc4334b7bcd Spec URL: https://raw.github.com/siwinski/rpms/8087060ca33da63e33848809b3215dc4334b7bcd/php-doctrine-cache.spec SRPM URL: http://siwinski.fedorapeople.org/SRPMS/php-doctrine-cache-1.3.0-2.fc20.src.rpm
[x]: Package does not generate any conflict. => comment ok. [x]: Dist tag is present (not strictly required in GL). [x]: %check is present and all tests pass. - Removed sub-packages -- This does make more sense and is easier this way... I don't know why I originally sub-packaged => Ok, simpler and cleaner. No blocker == APPROVED ==
THANKS for the review! New Package SCM Request ======================= Package Name: php-doctrine-cache Short Description: Doctrine Cache Owners: siwinski Branches: f19 f20 el6 InitialCC:
Git done (by process-git-requests).