Spec URL: https://raw.githubusercontent.com/pvalena/redhat/master/rubygems-mirror.spec SRPM URL: https://github.com/pvalena/redhat/blob/master/rubygems-mirror-1.1.0-1.fc24.src.rpm?raw=true Description: This is an update to the old `gem mirror` command. It uses net/http/persistent and threads to grab the mirror set a little faster than the original. Eventually it will replace `gem mirror` completely. Right now the API is not completely stable (it will change several times before release), however, I will maintain stability in master. Fedora Account System Username: pvalena
pvalena's scratch build of rubygems-mirror-1.1.0-1.fc24.src.rpm for f24-candidate completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11801483
This is my first review request and i am seeking a sponsor. My Informal Reviews: https://bugzilla.redhat.com/show_bug.cgi?id=1268450 https://bugzilla.redhat.com/show_bug.cgi?id=1268360
I'll take it for a review.
vondruch's scratch build of rubygems-mirror-1.1.0-1.fc24.src.rpm for f24-candidate completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11804389
* Package naming - This is tough one with regards to naming. The name according to naming guidelines should be "rubygem-rubygems-mirror". I personally would be fine with the shortened "rubygems-mirror", because this code actually used to be part of RUbyGems code base, priori it was extracted into RubyGems plugin and independent package. Nevertheless, after discussion with other packages, it seems that general consensus prefers consistency, i.e. the "rubygem-rubygems-mirror" name. Could you please rename the package? - Please also note that optionally, you could add: Provides: rubygems-mirror virtual provide into your package, so either rubygems-mirror or rubygem-rubygems-mirror will be installable in case of confusion. * Non-executable script - rpmlint complains about non-executable script: rubygems-mirror-doc.noarch: E: non-executable-script \ /usr/share/gems/gems/rubygems-mirror-1.1.0/Rakefile 644 /usr/bin/env - I'd suggest to drop the shebang from the Rakefile using something like: sed -i '/^#!\/usr\/bin\/env/ d' %{buildroot}%{gem_instdir}/Rakefile - Would be nice to suggest this change to upstream, since shebang in Rakefile is meaningless * Dot files removal - This is minor nit, but you can probably remove all the dot files by single sweep, e.g. you could use: %exclude %{gem_instdir}/.* * Separate license file - It would be nice to ask upstream to include license file [1]. Otherwise the package looks good. Please rename the package so I can approve it and sponsor you. [1] http://fedoraproject.org/wiki/Packaging:ReviewGuidelines#cite_note-24
Actually, one minor nit in addition. The summary and description are rather poor. You might try to collaborate with upstream on improving them. Nevertheless, this is not a blocker for a review, just a not.
I have renamed the package and fixed everything you mentioned, apart from the poor summary and description. I will look on that later. New Spec URL: https://raw.githubusercontent.com/pvalena/redhat/master/rubygem-rubygems-mirror.spec New SRPM URL: https://github.com/pvalena/redhat/raw/master/rubygem-rubygems-mirror-1.1.0-1.fc24.src.rpm Thank you.
Also, here are the pull requests: https://github.com/rubygems/rubygems-mirror/pull/42 https://github.com/rubygems/rubygems-mirror/pull/41
* rpmlint output - rpmlint complains: ./rubygem-rubygems-mirror.spec:19: W: unversioned-explicit-provides rubygems-mirror and that is because I was not specific enough (sorry). It is indeed good idea to version the provide, so the correct version is: Provides: rubygems-mirror = %{version}-%{release} Except the issue mentioned above, the package looks good, therefore I APPROVE the package and I'm going to sponsor you. Please fix the above mentioned issue and follow with the import according to instructions [1]. [1] https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rubygem-rubygems-mirror