Code review comment for lp:~gary-lasker/software-center/recommended-installed-feedback

> Hi Gary, thanks for this updated branch.
>
> I just reviewed it and it looks very nice now. Some comments below:
>
>
> I like that the recommendations widgets do no longer get a catview, giving
> them the db and propertieshelper
> looks much nicer now.
>
> 395 class FlowableGrid(Gtk.Fixed):
> 396
> 397 MIN_HEIGHT = 100
> 398
> 399 + __gsignals__ = {
> 400 + "application-activated": (GObject.SignalFlags.RUN_LAST,
> 401 + None,
> 402 + (GObject.TYPE_PYOBJECT, ),
> 403 + ),
> 404 + }
> 405 +
>
> It seems that the FlowableGrid is pretty generic (i.e. can hold other items
> than application tiles too) so this signal probably is better put into the
> TileGrid class? Or am I missing something here? I can do this during the merge
> if you agree with moving it.

Yes! Thank you for noticing that. I just overlooked moving it down to the new subclass, but you are correct, that's where it belongs.

>
> I think its great that you added the docstring:
> 416 + def add_tiles(self, properties_helper, docs, amount):
> 417 + '''Adds application tiles to an ApplicationTileGrid:
> 418 + properties_help = an instance of the PropertiesHelper object
> 419 + docs = xapian documents (apps)
> 420 + amount = number of tiles to add from start of doc range'''
> here! It might be worth taking a quick look
> http://www.python.org/dev/peps/pep-0257/ but there seems to be no real
> convention that everyone in the python community is following, so to a good
> extend its a matter of taste :) I personally like using a "--" as the keyword
> seperator there, e.g. "doc -- xapian docuemnts" better than using the "=" as
> initially my brain read it as code until I noticed the surrounding '''. But
> thats just a tiny detail and its good to have the docstring.
>

I am with you completely. I just cut and pasted from another docstring and didn't notice the "=" signs. I would much prefer something different, and "--" seems just fine to me.

> I like the way the "more" button is now handled, much nicer than in the
> previous code.

Thanks! I like all of this better too. :) I'll make the updates now.