[Bug 1116028] Review Request: rubygem-elasticsearch-transport - Elasticsearch transport

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #8 from Eduardo Mayorga <e@xxxxxxxxxxxxxxxx> ---
(In reply to Steve Traylen from comment #7)
> (In reply to Eduardo Mayorga from comment #5)
> 
> > 
> > Issues:
> > =======
> > - It is not necessary to include %{gem_spec} both in the main package and in
> > the doc subpackage, since the doc subpackage Requires the main one (and thus
> > can only be present if the main one is). I would recommend leaving
> > %{gem_spec} just in doc subpackage.
> > 
> 
> I left the %{gem_spec} in the main package. It's needed for gem
> to operate. Dropped from doc package.

OK.

> > - There is no such file as %{gem_instdir}/.gitignore in buildroot, so you
> > can get rid of
> > %files
> > ...
> > %exclude %{gem_instdir}/.gitignore
> > 
> 
> ?
> 
> It is there, dropping the explicit %exclude.
> 
> Error: Installed (but unpackaged) file(s) found:
>    /usr/share/gems/gems/elasticsearch-transport-1.0.4/.gitignore

You're right. This was an overlook on my side.

It's OK.

> > - The timestamps of source tarball in the source rpm are not preserved. You
> > need to download it using the -N argument for wget or -R for curl in your
> > build environment (SOURCES directory).
> > 
> 
> ack, change made.

OK.

> > - There seems to be a bug in the test suite. This is the output I get
> > [makerpm@localhost test]$ ruby test_helper.rb 
> > test_helper.rb:26:in `block in <main>': uninitialized constant Elasticsearch
> > (NameError)
> > /usr/share/rubygems/rubygems/core_ext/kernel_require.rb:55:in `require':
> > cannot load such file -- shoulda-context (LoadError)
> > 	from /usr/share/rubygems/rubygems/core_ext/kernel_require.rb:55:in `require'
> > 	from test_helper.rb:30:in `<main>'
> > 
> 
> So the this is just saying the gem 'rubygem-shoulda-context' is not
> installed.
> I could add it to the requirements of the -doc subpackage but once installed
> you only run into the next one that rubygem-turn is not installed and 
> this is not available.

It seems that's not the problem, because I'm unable to run the test even having
that gem installed.

> I guess this comes down to should the tests be included at all if they 
> don't work... Given they have been marked as doc anyway they don't have to
> work I would say, they are still present as a reference.
> 
> I will look at packaging rubygem-turn though.
> 
> Looking at the ruby guidelines it's unclear as to if test suites
> should be included, only that they should be run in possible which is 
> currently not the case.

Yeah, it's ok to leave it as it is for now.

> Spec URL:
> http://cern.ch/straylen/rpms/rubygem-elasticsearch-transport/rubygem-
> elasticsearch-transport.spec
> SRPM URL:
> http://cern.ch/straylen/rpms/rubygem-elasticsearch-transport/rubygem-
> elasticsearch-transport-1.0.4-2.fc20.src.rpm

Please update the SRPM URL.

> In addition I will once this package is okay look at updating the other
> elasticsearch reviews. I expect all of your comments may well apply
> there also.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]