[sugar] review: Daf's view branch

Dafydd Harries dafydd.harries
Thu Jun 5 09:37:47 EDT 2008


Ar 05/06/2008 am 12:40, ysgrifennodd Guillaume Desmottes:
> Just few comments:
> 
> 
> +class ActivityViewHandler(object):
> This class should have member_added and member_removed methods as its
> dummy implementation in test_model. A FIXME would be enough for now.
> We should probably have a properties_changed method too.

Agreed.

> +        self.activity_views = {}
> A comment explaining the type of the keys and values would be good.
> 
> 
> I think you're right, we could stop to support multi actions in queries
> (as you already dropped them in views)

Ok.

> In test_model.py you have some: #self.assertEquals(42, context) that
> could be removed.

Oops, right.

> Same for  #self.assertEquals([activity2], found_log.
> 
> 
> What's your test.py hack? Didn't we already merge the one making error
> message more informative?

Hmm, I thought we had merged it too. Perhaps I made a mistake.

> Except that looks good. I like how you refactored query/view parsing
> code and the general design of views.

Thanks!

I think the main missing aspects are:

 - random views
 - correct view size handling
 - stress tests (I have some code lying around for older iterations of the
   model which we can probably use)

-- 
Dafydd



More information about the Sugar-devel mailing list