[Bug 684835] Review Request: deltacloud-core - Deltacloud REST API server

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

 



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=684835

--- Comment #4 from Michal Fojtik <mfojtik@xxxxxxxxxx> 2011-03-15 12:35:34 EDT ---
(In reply to comment #3)
> * Cleaning
>   - "rm -rf %{buildroot}" at the top of %install and %clean sections
>     is no longer needed:
>     https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

Fixed.

> 
> * Duplicated BuildRequires
>   - BuildRequires: rubygem(json) is duplicated

Removed.

> 
> * Test suite
>   - It is interesting is that following commands provides different output:
>       1) testrb tests/*_test.rb tests/drivers/mock/*_test.rb
>       2) testrb tests/drivers/mock/*_test.rb tests/*_test.rb
>       3) rake test
>     while they should be equivalent.

??


> * Missing runtime Requires:
> 
>   [vondruch@dhcp-25-40 result]$ deltacloudd -i mock
>   Starting Deltacloud API :: mock :: http://localhost:3001/api
> 
>   ERROR: no such file to load -- rack/accept

I moved some of BuildRequires to Requires. Thanks for pointing me on this.

>   - Please compare with the BuildRequires which are sufficient. There are very
>     probably missing nokogiri, rake-accept and may be others.
> 
> * Deleting the %{_builddir}
>   - It is bad practice to delete %{_builddir} in installation step. It is not
>     harmful in this particular case, but once there would be binary extensions
>     it may cause troubles. This is coming from Ruby guidelines:
> 
> "Finally at %install stage the whole tree under the directory created at %prep
> stage should be copied (not moved) to under %{buildroot}%{gemdir}.
> 
>     When all tree under the directory created at %prep stage is moved to under
>     %{buildroot}, find_debuginfo.sh will complain that the corresponding source
>     files are missing."

I removed that rmdir, but for this particular case I don't think this is
necessary, since this package don't have any binary extensions (so no
debug-info is needed)


> * Garbage in support folder?
>   - It seems that folder /usr/share/deltacloud-core/support contains some
>     unnecessary stuff, potentially garbage? There is only "deltacloud-core"
>     which is later moved into %{_initdir}, the rest should not be installed,
>     nor it should be part of the gem IMO.

Hmm this one is really weird, I check support folder and it's part of %doc
subpackage.


> * Upstream
>   - The upstream package is not available yet. Please synchronize the release
>     with upstream.

Will do.


> * Documentation
>   - COPYING file should be marked as %doc.
>   - Rakefile should be moved into doc subpackage, since it is not required by
>     runtime

Files marked and moved Rakefile to %doc.

> 
> * MUST: A package must own all directories that it creates.
>   - Package does not own the %{app_root}/public directory (at least it stays on
>     my system after uninstall).

Marked with %dir

---------------------- rev 4 -----------------------

Spec URL: http://mifo.sk/fedora/deltacloud-core/master/deltacloud-core.spec
SRPM URL:
http://mifo.sk/fedora/deltacloud-core/master/deltacloud-core-0.2.0-4.fc14.src.rpm

-- 
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.
_______________________________________________
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]