Bug 784857 - Review Request: rubygem-openstack-compute - OpenStack Compute Ruby API
Summary: Review Request: rubygem-openstack-compute - OpenStack Compute Ruby API
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-26 13:46 UTC by Bohuslav "Slavek" Kabrda
Modified: 2012-01-27 13:37 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-01-27 13:37:17 UTC
Type: ---
vondruch: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Bohuslav "Slavek" Kabrda 2012-01-26 13:46:46 UTC
SPEC: http://bkabrda.fedorapeople.org/pkgs/openstack-compute/rubygem-openstack-compute.spec
SRPM: http://bkabrda.fedorapeople.org/pkgs/openstack-compute/rubygem-openstack-compute-1.1.5-1.fc16.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3734637

Ruby Openstack Compute binding for the v1.0 OSAPI.
Currently supports both v1.0 and v2.0 (keystone) auth.

Comment 1 Vít Ondruch 2012-01-26 14:17:24 UTC
I'll take it for a review.

Comment 2 Vít Ondruch 2012-01-26 15:47:22 UTC
* Version in patch name is questionable
  - If the patch is not accepted by upstream for next release, you will need
    either to rename the patch or remove the %{version} macro anyway,
    so I would suggest to not use macro in this case

* The test suite should not be carried by resulting gem
  - Although I appreciate that you are executing test suite, I suggest its
    unpacking just in %check section, which prevents its inclusion in
    resulting package.

    %check
    tar xzvf %{SOURCE1} -C .%{geminstdir}

* MiniTest instead of TestUnit
  - Have you considered using Minitest, which is now default in Ruby, instead of
    test-unit?

* rpmlint
  - Please fix following rpmlint error:

    rubygem-openstack-compute.noarch: E: non-executable-script \
      /usr/share/gems/gems/openstack-compute-1.1.5/lib/openstack/compute.rb \
      0664L /usr/bin/env

Comment 3 Bohuslav "Slavek" Kabrda 2012-01-27 06:54:49 UTC
(In reply to comment #2)
> * Version in patch name is questionable
>   - If the patch is not accepted by upstream for next release, you will need
>     either to rename the patch or remove the %{version} macro anyway,
>     so I would suggest to not use macro in this case
> 

True, done.

> * The test suite should not be carried by resulting gem
>   - Although I appreciate that you are executing test suite, I suggest its
>     unpacking just in %check section, which prevents its inclusion in
>     resulting package.
> 
>     %check
>     tar xzvf %{SOURCE1} -C .%{geminstdir}
> 

Done.

> * MiniTest instead of TestUnit
>   - Have you considered using Minitest, which is now default in Ruby, instead
> of
>     test-unit?
> 

Yes, tests work the same way with minitest, so I switched to it.

> * rpmlint
>   - Please fix following rpmlint error:
> 
>     rubygem-openstack-compute.noarch: E: non-executable-script \
>       /usr/share/gems/gems/openstack-compute-1.1.5/lib/openstack/compute.rb \
>       0664L /usr/bin/env

Fixed.

SPEC: http://bkabrda.fedorapeople.org/pkgs/openstack-compute/rubygem-openstack-compute.spec
SRPM: http://bkabrda.fedorapeople.org/pkgs/openstack-compute/rubygem-openstack-compute-1.1.5-2.fc16.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3738691

Comment 4 Vít Ondruch 2012-01-27 08:08:53 UTC
I have no other objections => APPROVED

Comment 5 Bohuslav "Slavek" Kabrda 2012-01-27 11:46:21 UTC
Thank you for your review!

New Package SCM Request
=======================
Package Name: rubygem-openstack-compute
Short Description: OpenStack Compute Ruby API
Owners: bkabrda
Branches: 
InitialCC:

Comment 6 Gwyn Ciesla 2012-01-27 13:11:56 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.