Spec URL: https://www.happyassassin.net/extras/reviews/php-irods/php-irods.spec SRPM URL: https://www.happyassassin.net/extras/reviews/php-irods/php-irods-3.3.0-0.1.beta1.fc21.src.rpm Description: PRODS is a PHP client API for iRODS (http://www.irods.org). It talks to an iRODS server directly via sockets with a native iRODS XML protocol. Fedora Account System Username: adamwill
Packaging this purely for OwnCloud - it's bundled with OwnCloud 6.0.0+ . It lets OwnCloud use an iRODS installation as an external file source. I don't have access to any iRODS installations so I can't actually test it, but the package is very simple. I followed the Debian package's approach heavily: http://packages.debian.org/sid/php-irods-prods in particular, just leaving out large chunks which aren't really needed. The web/ directory contains a webapp for accessing iRODS installs, which is all fine and dandy but OC doesn't need it and I've no interest in it. I structured the package the way I did - with the src.rpm called 'php-irods' but no binary 'php-irods' package, only a 'php-irods-prods' subpackage - to allow for the possibility of someone producing a php-irods-web or whatever if they want. As far as prods - the actual PHP library - goes, I left out the tests (as they need an entire iRODS deployment on the environment where you're testing, which we can't really accommodate, I don't think...) and the tutorials and a few other bits of garnish briefly noted in the spec file, much as Debian did; again, these can be added later if someone has a use for them. There's a config file which only covers SSL stuff in /usr/share/php/irods/prods ; if anyone really cares I could move it to /etc and add a small patch so it finds it there. Or just nuke it (or ship it as documentation) and let people add it themselves if they want it.
Above URL is 404.
sigh, that'll teach me to eyeball it. drop the extras/ : https://www.happyassassin.net/reviews/php-irods/php-irods.spec https://www.happyassassin.net/reviews/php-irods/php-irods-3.3.0-0.1.beta1.fc21.src.rpm
Open to suggestions for alternative paths too, wasn't really sure what to use.
Damn... I think I have commented this review... probably forget to press the submit button :( Yes, please move the config file to /etc About the "web" part, if someone want it, it will not have to be in the php namespace, as this is not a library (so p.e. "irods" will be a correct name) But I'm fine with current naming (php-irods is ok too) I also think "tutorials" and "utilities" should go in %doc, so the full "prods" tree will be provided (warning: clean the .svn dir). I prefer to see "Fix: wrong-file-end-of-line-encoding" in %prep (and using perl is probably a big hammer when sed is enough) %{dist} => %{?dist} (new fedora-review complains about this, and is right) Even if LICENSE.txt is identical to prods/src/LICENSE.txt, you should rather use the second one, which really apply to prods library.
Updated: https://www.happyassassin.net/reviews/php-irods/php-irods.spec https://www.happyassassin.net/reviews/php-irods/php-irods-3.3.0-0.2.beta1.fc21.src.rpm All of the above addressed, couple of other small things fixed, and I laid the package out more like Debian; there are no great choices for layout, and preserving upstream's layout under /usr/share/php seems like the least worst option, and the fact that it's consistent with Debian is a bonus. We more or less have to invent the top-level directory name since the upstream zip doesn't have one: this matches the packagename and the name Debian uses, but OwnCloud called their top-level directory irodsphp, so we'll have to carry a patch or convince upstream to rename the directory (which we have more chance of doing if we and Debian both use the same name, I guess).
Created attachment 845702 [details] phpci.log phpcompatinfo version 2.26.0.
Created attachment 845703 [details] review.txt Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30 Command line :/usr/bin/fedora-review -b 1047478
[!]: Requires correct, justified where necessary. From phpcommpatinfo report: missing php-libxml, php-pcre, php-spl (Core/standard are obviously not extension) Requires: php(language) >= 5.3.0 Can you explain where you found this information ? I think having php-irods.src.rpm and php-irods-prods.noarch.rpm is a bit ugly... What do you think of simply "php-irods" ? (I will approved the package whatever your choice is).
I can only have got the PHP version info from phpci; perhaps I ran it against the entire tree and something in the tests or tutorials requires 5.3. I forgot to add the 'missing' module requires to this package, oops, will do. I really don't freaking know what to do with the name any more, this project is such a pain in the ass. Debian calls it php-irods-prods . OwnCloud bundles it in the directory irodsphp . I've got three different names in different bits of my spec. I don't fucking know. Can we throw a dart at something and do whatever it says?
OK, let's just call it php-irodsphp as discussed on IRC, it's a reasonable guess as to the 'project name' as it's in the upstream URL (and it's what OwnCloud decided on). New build with all issues addressed (I hope): https://www.happyassassin.net/reviews/php-irodsphp/php-irodsphp.spec https://www.happyassassin.net/reviews/php-irodsphp/php-irodsphp-3.3.0-0.3.beta1.fc21.src.rpm
Yes, Naming is really simpler. [x]: Requires correct, justified where necessary. No blocker. == APPROVED ==
New Package SCM Request ======================= Package Name: php-irodsphp Short Description: PHP client API for iRODS Owners: adamwill Branches: f20 el6 InitialCC:
Git done (by process-git-requests).
Built for Rawhide, will build for other branches as needed by OwnCloud (I requested f20 and el6 as it's possible we'll bump to OC 6 on those).
Package Change Request ====================== Package Name: pkgname New Branches: f19 epel7
Invalid package name, no owners.
It's a *modification* request. I don't want to change the owners. This system is freaking absurd. The bug report is for a package. I have to copy the package name from the bug summary to the change request? really? I mean, it's just an invitation to get something wrong when you're trying to get a perfectly simple change done to a bunch of packages. It's really arsing annoying. Package Change Request ====================== Package Name: php-irodsphp New Branches: f19 epel7
Package Change Request ====================== Package Name: php-irodsphp New Branches: f19 epel7 Owners: adamwill