[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