Code review comment for lp://staging/~danilo/launchpad/complex-forms

Revision history for this message
Данило Шеган (danilo) wrote :

У сре, 02. 09 2009. у 13:08 +0000, Jeroen T. Vermeulen пише:
> Sheer poetry. My main gripe—because I just _had_ to find one!—is that
> you didn't turn the icons into sprites. Any chance of getting that
> in?

Sure, done that now.

> Also, in productseries-translations-settings.pt, the TAL still
> composes links to branches the painful way, complete with icons. I
> was probably guilty of that myself in the case of the export branch.
> Can't we use fmt:link for these?

Yeah, I did this, but that one of yours caused me the most trouble: you
set an ID on it, and then used that in a pagetest. When using fmt:link,
you can't do that, and there's no getLink() on anything other than the
browser. Doing browser.open(link['href']) caused more trouble then it
helped so I resorted to using browser.getLink(url=branch_name) which
worked, but it was an exercise in its own right.

> All this will break some pagetests, but I heard it will make our hair
> glow with natural beauty and make our cars use less gasoline.

If our cars use less gasoline, what are we going to do with what we are
left on Earth? It's not like we can eat it so we should better spend it
all.

> As for the whitespace cleanup: are you out to prove that emacs is good
> for this? Not that I'd mind if you were. :-)

Nah, I wasn't actually expecting you to review this :)

« Back to merge proposal