| Summary: | Review Request: rubygem-virt - Simplied interface to use ruby the libvirt library | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Lukas Zapletal <lzap> |
| Component: | Package Review | Assignee: | Miroslav Suchý <msuchy> |
| Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, lhh, msuchy, notting, ohadlevy, package-review, vondruch |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2013-07-04 16:07:08 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
|
Description
Lukas Zapletal
2011-01-31 15:45:46 UTC
rubygem-virt.src: W: spelling-error %description -l en_US Simplied -> Implied, S implied, Simplified %description does not sound as english to me :) Description have to end with dot. rubygem-virt.src: W: no-%prep-section rubygem-virt.src: W: no-%build-section This section should be present, and should be empty. And just courious question: why did you start new project, when there is for long time: http://libvirt.org/ruby/ ? Hmm, did I understood correctly, that it provides simplified interface to ruby-libvirt? Otherwise it seem OK from the first view. You are missing requires for ruby-libvirt or more precisely for ruby(libvirt). Since you install Rakefile you probably should require rubygem-bundler as well. Note, that I stop searching after I find those two missing dependecies, so there may be others. Thanks Mirek and I am sorry it took so long to get back to it. I fixed all your remarks. And I was really missing the dependency. Bundler is not necessary, its just the one you mentioned. http://koji.fedoraproject.org/koji/taskinfo?taskID=3410848 $ rpmlint rubygem-virt-0.1.0-1.fc15.src.rpm rubygem-virt.src: W: spelling-error Summary(en_US) libvirt -> liberty rubygem-virt.src: W: spelling-error %description -l en_US libvirt -> liberty 1 packages and 0 specfiles checked; 0 errors, 2 warnings. SPEC & SRPM: http://static.zapletalovi.com/fedora/rpm/rubygem-virt/0.1.0-2 @Ohad - Would you mind testing the latest build on Fedora 15 or 16 for me? It's a rawhide build... http://koji.fedoraproject.org/koji/getfile?taskID=3410848&name=rubygem-virt-0.1.0-1.fc17.noarch.rpm If I may, I would suggest the following: * Remove unnecessary ruby_sitelib and ruby_sitearch globals, since they are not used for this gem. * The require should preferably use the virtual providers every Ruby package should have, i.e. use Requires: ruby(libvirt) and BuildRequires: ruby(rubygems) * The package should reference required ruby(abi) * The BuildRoot is obsolete * The clean section is not required * The %defattr macros are obsolete * It would be nice to use -doc subpackage for larger documentation * I always suggest to execute the test suite in the %check section to protect you, your users and possibly somebody else who will need to rebuild your package BTW have you updated the release number nor not? Or am I doing review on wrong srpm? But I took one from Koji [1] you reference ... [1] http://koji.fedoraproject.org/koji/taskinfo?taskID=3410848 * The BuildRoot is obsolete * The clean section is not required * The %defattr macros are obsolete Well unless you plan to do epel release. And I'm quite sure Lukas will be very interested to push it into epel as well. I second #5 and beside that: * how it comes that bundler is not necessery. The Rakefile is still there. And it requires bundler. Since rpm has no soft dependecies you should list all requires, even if they are not neede for some tasks. Or you can remove the Rakefile. AFAIK it is not needed in runtime. Yes, that is true, however %defattr is not required for any version of EPEL as far as I know. Not sure about the others. Neither Rake nor Bundler are required for proper Gem functionality. Actually you have no chance to simply call the rake for gem, typically we include the Rakefile just in -doc subpackage. ping. any progress here? I guess I will have to start over with the new gem2rpm script. So many changes. You probably did not wanted to change component. Reseting back. Ping? Guys, I am taking Mirek's version from our thirdparty repo: RPM: http://kojipkgs.fedoraproject.org//work/tasks/6184/4786184/rubygem-virt-0.2.1-3.fc19.noarch.rpm SRPM: http://kojipkgs.fedoraproject.org//work/tasks/6184/4786184/rubygem-virt-0.2.1-3.fc19.src.rpm SPEC: https://raw.github.com/lzap/spec_reviews/master/rubygem-virt.spec Upstream provide tests. You should run in %check phase. Ok to run tests we need a rubygem jeweler which is not in Fedora yet: https://github.com/ohadlevy/virt/blob/master/Rakefile http://kojipkgs.fedoraproject.org//work/tasks/6216/4786216/root.log I guess I will leave this effort on our baseos guys. Feel free to take this bug and finish the packaging. (In reply to comment #17) > Ok to run tests we need a rubygem jeweler which is not in Fedora yet: Don't use Rake to execute the test suite and you'll be fine. And the reason is in your quote as well as in guidelines [1]. [1] https://fedoraproject.org/wiki/Packaging:Ruby#Running_test_suites ping! any progress here? Guys, we dropped support of rubygem-virt in Foreman cos we are using rubygems libvirt and fog now. This project seems to be dead now. I am closing this review, thank you for help and effort on this. |