[Bug 746031] Review Request: rubygem-aeolus-cli - Command-line interface for working with the Aeolus cloud suite

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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

--- Comment #3 from James Laska <jlaska at redhat.com> 2011-10-17 16:34:52 EDT ---
(In reply to comment #2)
> OK thank you for the review. Updated package:
> 
> Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=3437829
> Spec URL: http://mo.morsi.org/files/rpms/rubygem-aeolus-cli.spec
> SRPM URL:
> http://mo.morsi.org/files/rpms/rubygem-aeolus-cli-0.1.0-5.fc15.src.rpm

rpmlint checks out okay again.

> > > [ FAIL ] MUST: The sources used to build the package must match the upstream
> > >          source, as provided in the spec URL. Reviewers should use md5sum for this task.
> > >          If no upstream URL can be specified for this package, please see the
> > >          http://fedoraproject.org/wiki/Packaging/SourceURL for how to deal with
> > >          this.
> > 
> > The instructions included in the .spec for creating the Source URL do not work.
> > 
> > # git clone  git://git.fedorahosted.org/aeolus/conductor.git
> > # git checkout next
> > # cd services/image_factory/aeolus-image
> > # rake gem
> > # grab image_factory_console-0.0.1.gem from the pkg subdir
> > 
> 
> Updated these to reflect the new rubygem-aeolus-cli source location

Works good on an F16 system.  It was failing on my f15 box since rubygem-rdoc
is not available there.  Does 'rubygem-rdoc' need to be a BuildRequires?

# git clone git://github.com/aeolusproject/aeolus-image.git
# rake gem
# grab aeolus-cli-0.1.0.gem from the pkg subdir

./pkg/aeolus-cli-0.1.0.gem

> > > [ FAIL ] MUST: Packages must not own files or directories already owned by
> > >          other packages. The rule of thumb here is that the first package to be
> > >          installed should own the files or directories that other packages may rely
> > >          upon. This means, for example, that no package in Fedora should ever share
> > >          ownership with any of the files or directories owned by the
> > >          <code>filesystem</code> or <code>man</code> package. If you feel that you have
> > >          a good reason to own a file or directory that another package owns, then please
> > >          present that at package review time. (refer to
> > >          http://fedoraproject.org/wiki/Packaging/Guidelines#FileAndDirectoryOwnership)
> > 
> > 
> > Transaction Check Error:
> >   file /usr/bin/aeolus-image from install of
> > rubygem-aeolus-cli-0.1.0-4.fc16.noarch conflicts with file from package
> > rubygem-aeolus-image-0.0.1-5.fc16.noarch
> >   file /usr/share/man/man1/aeolus-image-build.1.gz from install of
> > rubygem-aeolus-cli-0.1.0-4.fc16.noarch conflicts with file from package
> > rubygem-aeolus-image-0.0.1-5.fc16.noarch
> >   file /usr/share/man/man1/aeolus-image-list.1.gz from install of
> > rubygem-aeolus-cli-0.1.0-4.fc16.noarch conflicts with file from package
> > rubygem-aeolus-image-0.0.1-5.fc16.noarch
> > 
> 
> 
> OK this file conflict was fixed w/ the latest version of rubygem-aeolus-image
> (now in updates-testing). I added the aeolus-image dependency to the aeolus-cli
> specfile, specifying the minimum version required.

Thanks, this appears to be resolved in the latest version.

> > > [ WARN ] SHOULD: The reviewer should test that the package functions as
> > >          described. A package should not segfault instead of running, for example.
> > 
> > I have no idea how to test this ... any suggestions?
> 
> 
> Yes, a surface verification would be to make sure you can run
> /usr/bin/aeolus-image --help without any errors / segfaults. That is probably
> 

No obvious problems identified here.

> > > [ FAIL  ] The package must provide rubygem(%{gemname}) where gemname is the name from the Gem's specification. For every dependency on a Gem named gemdep, the package must contain a Requires on rubygem(%{gemdep}) with the same version constraints as the Gem 
> > 
> > The package does provide the rubygem: "Provides: rubygem(%{gemname}) =
> > %{version}"
> > 
> > However, it does not require the same version as expected: "Requires:
> > rubygem(%{gemname}) = %{version}"
> > 
> > It does seem odd for a package to require itself.
> 
> 
> Note the guidelines state "___for every dependency___ the package must contain
> a requires..."
> 
> This dependency is not for the package itself.

That makes a lot more sense :)

Some additional comments from a rubygem review that vondruck performed
previously ...

* Please execute test suite if available upstream.
* I would suggest to move following files into -doc subpackage:

%{geminstdir}/Gemfile
%{geminstdir}/Rakefile
%doc %{geminstdir}/minitest
%doc %{geminstdir}/test
%doc %{geminstdir}/spec
%doc %{geminstdir}/%{gemname}.gemspec
%doc %{geminstdir}/CHANGELOG.md
%doc %{geminstdir}/Guardfile

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.