[Bug 1242056] Review Request: rubygem-chake - serverless configuration management tool for chef

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

 



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

Vít Ondruch <vondruch@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vondruch@xxxxxxxxxx



--- Comment #15 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
(In reply to Athos Ribeiro from comment #13)
> (In reply to Paulo Andrade from comment #12)
> >   Hi Athos,
> 
> Hi Paulo, thank you for the feedback.
> 
> > 
> >   Please comment on this:
> > W: simplecov not installed, we won't have a coverage report
> > maybe it needs this build requires?
> > rubygem-simplecov.noarch : Code coverage analysis tool for Ruby 1.9
> 
> simplecov is a gem to measure test coverage. I don't see any advantage on
> measuring test coverage during the build step since the output would be
> ignored. It would also generate a "coverege" directory which would have to
> be removed. see https://github.com/colszowka/simplecov for reference.

That is right. We typically omit code coverage runs from the test suite. This
is upstream stuff, we don't generally care about it.

Less dependencies for package build is always better.

> > 
> >   I see the -doc package is installing files under:
> > /usr/share/gems/gems/chake-0.13/
> > I believe this is incorrect. Are you sure the main package runs
> > without the files installed there?
> 
> Yes,
> 
> the -doc files under /usr/share/gems/gems/chake-0.13/ are
> 
> /usr/share/gems/gems/chake-0.13/examples
> 	which is a directory with examples on how to use chake
> 
> /usr/share/gems/gems/chake-0.13/spec
> 	which is a directory with integration tests
> 
> /usr/share/gems/gems/chake-0.13/ChangeLog.md
> /usr/share/gems/gems/chake-0.13/README.md
> 	upstream changelog and readme files
> 
> /usr/share/gems/gems/chake-0.13/Rakefile
> 	Which is a file for rake (make for ruby, so this would be something like a
> Makefile)
> /usr/share/gems/gems/chake-0.13/Gemfile
> 	File defining all dependencies of the project
> /usr/share/gems/gems/chake-0.13/chake.gemspec
> 	File that defines the gem
> 
> These files usually go in the -doc in other packaged gems.


This list and the split between main package and -doc subpackage looks
reasonable to me.

> > 
> >   Please also comment on the directory:
> > /usr/share/gems/doc/chake-0.13/ri
> > is it really required by the -doc package? Either way, what is
> > installing in /usr/share/gems/doc/chake-0.13/ should be installed
> > in /usr/share/doc/chake
> 
> The ri directory contains the ri documentation for the gem. ri documentation
> can be browsed through ri calls on the command line.
> 
> About being installed in /usr/share/doc/chake:
> 
> As you can see here (f23):
> $ dnf repoquery --repoid=fedora -l rubygem-*-doc | grep -i
> "^/usr/share/gems/doc/" | cut -d/ -f6 | uniq | wc -l
> 531
> $ dnf repoquery --repoid=fedora rubygem-*-doc | grep rubygem | wc -l
> 533
> 
> only 2 out of 533 gems do not install documentation in /usr/share/gems/doc,
> shouldn't I also follow this pattern?


The generated documentation must be available at these locations, since the
rubygems tooling expects it to be there. For example "gem server" can server
the documentation from there. Similarly, ther "ri" command knows these location
to server the documentation properly.

> > 
> >   About the fonts, I believe the bug report is
> > https://bugzilla.redhat.com/show_bug.cgi?id=1224715 and it will
> > not get any attention, as the linked, related upstream report
> > is closed https://github.com/rdoc/rdoc/issues/186 as they
> > apparently had a different idea about it.
> 
> I see...
> 
> > Please check that just adding a 'Requires: lato-fonts' is not
> > enough to display the documentation, and if not enough, please
> > check what kind of patch could be done, apparently only in the
> > *.css files.
> > I understand it is replicated in more than 500 packages, but
> > that is not correct :(
> 
> I removed the generated fonts, required the two packages wich contain them
> and added links to these files, how about that? I am also checking with Ruby
> SIG if that approach would be feasible for all the other 500+ -doc packages.

While I agree this should be fixed, I don't think it is good idea to fix only
single gem. I would prefer all or nothing.


> > 
> >   Please check https://fedoraproject.org/wiki/Packaging:Ruby
> > the template there installs documentation under:
> > $ rpm -E %_defaultdocdir
> > /usr/share/doc
> > what should be done by the sample command in the sample spec:
> > rdoc --op %{buildroot}%{_defaultdocdir}/%{name}
> 
> The example shown is for ruby applications, there are other instructions for
> packaging gems there. As mentioned above, there is a macro defined with the
> path for gems documentations in
> https://fedoraproject.org/wiki/Packaging:Ruby#Macros

Agree, this is correct explanation.

> >   About the reviews with no longer srpm or spec, what you did
> > is fine, just comment about it in the bug report :)

Please note that there is process for stalled reviews:

https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews

-- 
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://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




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