Bug 837008

Summary:	Review Request: rubygem-ruby-libvirt - Ruby bindings for LIBVIRT
Product:	[Fedora] Fedora	Reporter:	Bohuslav "Slavek" Kabrda <bkabrda>
Component:	Package Review	Assignee:	Vít Ondruch <vondruch>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	notting, package-review, tdawson, vondruch
Target Milestone:	---	Flags:	vondruch: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-07-03 13:32:53 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Bohuslav "Slavek" Kabrda 2012-07-02 13:58:53 UTC

Spec URL: http://bkabrda.fedorapeople.org/pkgs/ruby-libvirt/rubygem-ruby-libvirt.spec
SRPM URL: http://bkabrda.fedorapeople.org/pkgs/ruby-libvirt/rubygem-ruby-libvirt-0.4.0-1.fc17.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4213157
Description: Ruby bindings for libvirt.
Fedora Account System Username: bkabrda

Comment 1 Bohuslav "Slavek" Kabrda 2012-07-02 14:00:15 UTC

Please note, that I have already asked upstream to fix the rpmlint warning about outdated license file:

https://bugzilla.redhat.com/show_bug.cgi?id=837006

Comment 2 Darryl L. Pierce 2012-07-02 14:06:26 UTC

Can you also remove the "-ruby-" from the package name since it's redundant?

Comment 3 Bohuslav "Slavek" Kabrda 2012-07-02 14:14:47 UTC

(In reply to comment #2)
> Can you also remove the "-ruby-" from the package name since it's redundant?

No, I don't think that's a good idea, as there is a "libvirt" gem [1] - and if we ever need to package it, this would turn out to be a problem.

[1] https://rubygems.org/gems/libvirt

Comment 4 Vít Ondruch 2012-07-03 05:48:03 UTC

I'll take it for a review.

Comment 5 Vít Ondruch 2012-07-03 06:27:27 UTC

* Forgotten TODO?
  - Your spec file contains "# TODO: move the extensions". Seems to be some
    relict?

* Disabled test suite
  - Sad to see test suite disabled. Have you tried to confirm with upstream that
    root privileges are really needed?(In reply to comment #3)

These comments are just minor nits. I see no other issues => APPROVED

Comment 6 Bohuslav "Slavek" Kabrda 2012-07-03 07:36:30 UTC

Thank you for the review!

(In reply to comment #5)
> * Forgotten TODO?
>   - Your spec file contains "# TODO: move the extensions". Seems to be some
>     relict?
> 

Yep, I forgot it there. I will remove it before importing into dist-git.

> * Disabled test suite
>   - Sad to see test suite disabled. Have you tried to confirm with upstream
> that
>     root privileges are really needed?(In reply to comment #3)
> 

The tests directly communicate with libvirt, for which you always needs to be superuser. Moreover, there are some system files modified in /etc. Without it, tests would have no meaning, everything would need to be mocked.

> These comments are just minor nits. I see no other issues => APPROVED


New Package SCM Request
=======================
Package Name: rubygem-ruby-libvirt
Short Description: Ruby bindings for LIBVIRT
Owners: bkabrda
Branches: 
InitialCC:

Comment 7 Gwyn Ciesla 2012-07-03 11:56:05 UTC

Git done (by process-git-requests).

Comment 8 Troy Dawson 2014-12-18 23:35:27 UTC

Package Change Request
======================
Package Name: rubygem-ruby-libvirt
New Branches: epel7
Owners: tdawson

Comment 9 Gwyn Ciesla 2014-12-19 13:30:43 UTC

Git done (by process-git-requests).