On Tue, Sep 7, 2010 at 9:42 PM, Leonard Richardson
<email address hidden> wrote:
> Review: Approve
> It's been a while since I did anything with TALES, but I think I was able to follow this. I approve with only minor comments:
Thanks Leonard,
>
> line 48 "betteen"
Done
> Can 58 and 59 fit on one line?
Indeed.
> Why are lines 134 and 135 commented out? It seems like an important part of the test.
Indeed - it was a mistake. I've uncommented and reworked that line to
check that the flag is not set.
> Is it really OK to have the python: call on line 328? I remember that being discouraged.
It's OK if it's necessary - I couldn't find a way in TALES to index an
iterable (nor think of a better way to provide this via the view - ie.
construct a dict of first comments for all differences in the batch?
erg)... if you know one, please let me know.
Here's the incremental diff with the changes you recommended:
On Tue, Sep 7, 2010 at 9:42 PM, Leonard Richardson
<email address hidden> wrote:
> Review: Approve
> It's been a while since I did anything with TALES, but I think I was able to follow this. I approve with only minor comments:
Thanks Leonard,
>
> line 48 "betteen"
Done
> Can 58 and 59 fit on one line?
Indeed.
> Why are lines 134 and 135 commented out? It seems like an important part of the test.
Indeed - it was a mistake. I've uncommented and reworked that line to
check that the flag is not set.
> Is it really OK to have the python: call on line 328? I remember that being discouraged.
It's OK if it's necessary - I couldn't find a way in TALES to index an
iterable (nor think of a better way to provide this via the view - ie.
construct a dict of first comments for all differences in the batch?
erg)... if you know one, please let me know.
Here's the incremental diff with the changes you recommended:
http:// pastebin. ubuntu. com/490231/
Now onto the UI tweaks... thanks Leonard!