> Hi James,
>
> I think this looks good but the new functions/methods would benefit from
> some docstrings.
I will add some, thanks.
> Why 3 update() calls instead of just one passing a dict combining
> everything?
No reason really, I'll change it.
> > + $(".workitems_in_status").tablesorter({
> > + sortList: [[1,1],[0,0],[2,0]],
>
> Maybe a comment explaining these magic numbers?
Good idea.
> To make this even nicer you could add something like a
> valid_states_with_labels to report_tools (by zip()ping a list containing
> the lables with the existing valid_states there). :)
I'll do that. I need to see if we can import directly, because
passing it in the data dict would be a bit of a hassle.
> Hi James,
>
> I think this looks good but the new functions/methods would benefit from
> some docstrings.
I will add some, thanks.
> Why 3 update() calls instead of just one passing a dict combining
> everything?
No reason really, I'll change it.
> > + $(".workitems_ in_status" ).tablesorter( { [0,0],[ 2,0]],
> > + sortList: [[1,1],
>
> Maybe a comment explaining these magic numbers?
Good idea.
> To make this even nicer you could add something like a with_labels to report_tools (by zip()ping a list containing
> valid_states_
> the lables with the existing valid_states there). :)
I'll do that. I need to see if we can import directly, because
passing it in the data dict would be a bit of a hassle.
Thanks,
James