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

[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=674060

--- Comment #6 from Michal Fojtik <mfojtik at redhat.com> 2011-02-10 11:03:53 EST ---
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > * Check
> > >   - pushd ./%{geminstdir} is not stepping into the correct directory. 
> > >     The right one is: pushd %{buildroot}%{geminstdir}
> > 
> > AFAIK it's stepping to buildroot, but I replaced ./ with that macro just for
> > sure. Thanks for noticing that.
> > 
> > > * Shipping external project
> > >   - This gem bundles external project in support folder. On Fedora shipping
> > >     external project in the same package should (must) be avoided, see:
> > 
> > Ouch, sorry I built that gem in support directory so there was some leftovers.
> > It should be fixed now.
> 
> Still the same gem file is bundled => it is not fixed yet.

Sorry for that, seems like building this RPM inside support directory wasn't
good idea...

> 
> > > https://fedoraproject.org/wiki/Packaging/Guidelines#Bundling_of_multiple_projects
> > > 
> > > * Missing dependencies
> > >   - Since this is Sinatra application, there have to be specified 
> > >     appropriate dependencies:
> > > 
> > >     Require: rubygem(sinatra)
> > >     BuildRequire: rubygem(sinatra)
> > 
> > Require: rubygem(sinatra) was already here but it was missing in BuildRequire.
> > Should be fixed now.
> > 
> > >   - Missing rubygem-net-ssh dependency. Not sure if it is runtime 
> > >     or just development dependency
> > 
> > That's right, thanks for corrections.
> > 
> > > 
> > >   - There are missing plenty of others, such as rack. Please test using mock.
> > 
> > Should be fixed now. Note that other dependencies like 'rubygem-aws' are not
> > 'runtime' dependencies but they are optional. Eg. for using EC2 or other cloud
> > providers.
> > 
> > > * rpmlint output:
> > >   - Is this package ahead of upstream?
> > > 
> > >     $ rpmlint rubygem-deltacloud-core.spec 
> > >     rubygem-deltacloud-core.spec: W: invalid-url Source0:
> > >       http://gems.rubyforge.org/gems/deltacloud-core-0.2.0.gem 
> > >       HTTP Error 404: Not Found
> > >     0 packages and 1 specfiles checked; 0 errors, 1 warnings.
> > 
> > Yes, I'll need to wait with importing until this package will be available in
> > upstream (which should happen in next few days).
> > 
> > > 
> > > I am not going forward with this review, since I cannot build on my environment
> > > due to missing dependencies.
> > 
> > ======================= v0.2.0-2 ======================
> > 
> > Spec URL: http://mifo.sk/RPMS/deltacloud-core.spec
> > SRPM URL: http://mifo.sk/RPMS/deltacloud-core-0.2.0-2.fc14.src.rpm
> > 
> > Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2761635
> 
> * Rawhide build
>   - The package cannot be build against rawhide, since rawhide contains newer
>     Sinatra package and there are missing dependencies (tilt, may be others).
>     But this might very well be Sinatra package flaw, since tilt is required in
>     lib/sinatra/base.rb for sinatra 1.1.2 (hmm, no test suite executed during
>     Sinatra build, so no surprise :/ )
> 
> * Project name
>   - You have renamed the project, but I have some doubts about that. I believe
>     it should be discussed in Ruby-SIG. Are there some policies for shipping
>     Rails/Sinatra/Other Ruby applications? The reasons are bellow.
> 
>   - Is there any chance that any other gem will use the code in lib? If no,
> then
>     it just pollutes Ruby load path and should be avoided to prevent
> unnecessary
>     name collisions.

Well I guess all gems are separated and it will not pollute anything when it
will
not be explicitly required with 'require'.  

>   - This package should be manageable just by RPM. Managing by RubyGems has no
>     benefits for Fedora nor Ruby environment. Therefore please consider
>     providing package in tar.gz or similar form instead of gem.

I agree, this is something that needs to be discussed in Ruby-SIG.

> * Shipping external project
>    - This gem bundles external projects in lib/sinatra folder. On Fedora
>      shipping external project in the same package should (must) be avoided,
>      see:
> 
>     
> https://fedoraproject.org/wiki/Packaging/Guidelines#Bundling_of_multiple_projects

Well it ships some rack middleware, which is not packaged and those are just
single files
with our modifications. Rack-Accept is also our modification which is not
compatible with upstream
either fedora package.
Another things are OK I guess, url_for is also our source, not external one
(managed and created by
us).

> * Licensing
>   - lib/deltacloud/drivers/opennebula is GPLv2.1+

Yes, but since we switched code to Apache Incubator, all sources are now under
ASF licence.
I'll fix the licence in upstream. 

Will upload new version tomorrow.

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