On Mon, 2011-09-12 at 08:34 -0400, Tong Li wrote:
> I do not think using "lib" to reference anything is a good practice. It
> just causes problems. if not using lib, DC artifacts can be used by other
> gems. when some reference lib, then it actually won't work,  problem
> described in item 2 is very easy to fix, simply remove "lib/", then it will
> work just fine. If it is not an issue for DC, but it is not even a good
> practice beside the point of reuse.

Yes, I agree; and that will be fixed in due time. I just don't want to
spin another release candidate and hold another vote just for minor
fixes like this.

> for item3, when I used dc as a gem, used in my own gem, it did not work,

As I said, there is zero guarantee that using the dc server gem as a
library works.

> the method is undefined, even though integer.rb was required already, not
> sure what went wrong.

Hmm .. weird.

> for item4, the singularize method actually require i18n, can you check if
> your env actually has i18n, if you do, then you are probably using it
> without knowing you are.

This is from lib/deltacloud/core_ext/string.rb:

          def singularize
            return self.gsub(/es$/, '') if self =~ /sses$/
            self.gsub(/s$/, '')
          end
        
As you can tell, there's no dependency on i18n. As I said in my previous
mail, we do not use active_support or i18n, even though we have method
names in core_ext that follow commonly known method names from Rails.

> item 1 described in previous email was a bug. Please take a look.

As Michal explained, it's redundant code with no adverse effects. For an
array a, 'a.map' behaves the same as 'a.map { |x| x }', which is
perfectly fine for the use in views/instances/show.html.haml - certainly
something that should be cleaned up, but not a release stopper.

> for the slow performance, I can not be sure what is causing it. please take
> a look at this firebug capture, not sure why the API GET instances took
> 25.07s, not sure why it references include.js from tb.adurr.com.

That is very strange - there is no such load for me; there's also no
reference to either include.js or adurr.com. It seems this isn't coming
from Deltacloud.

If you google for tb.adurr.com, you'll find various posts. I didn't
follow them enough to determine if this is benign or malware, but would
probably warrant closer follow-up.

>  I am very
> sure that 0.3.0 won't take that long. Can not say it was because of thin or
> webrick, or browser version, when I took the snap shot of firebug, I was
> using latest firebug and firefox.

Very strange.

> this is to get all the instances.
> 
> 
> Another one for rendering a particular instance.

Did you mean to attach more details ?

thanks,
David


Reply via email to