Code review comment for lp:~dimitern/maas/bug-994887-nodes-listing-does-not-support-sorting

Revision history for this message
Martin Packman (gz) wrote :

The project much of this code comes from only has a license specified in setup.py (GPLv3, which is okay to redistribute as AGPLv3), but we probably want the copyright header in that file to read "Drew Yeaton" instead of/as well as Canonical.

A fair bit of that code wiggles my wtf-meter, is the best way of doing this really writing a custom template snippet parser (parse_tag_token... no wonder django templating is fragile), then constructing the link (SortableLinkNode.make_link) and html (SortableLinkNode.render) with string interpolation? I wouldn't trust this to, for instance, correctly escape '&' -> '&', but maybe I underestimate django magic.

Unlike John I don't worry about the headers being page reloads for sorting rather than fast JS things, this way it at least combines correctly the pagination, and has predictable behaviour.

review: Abstain

« Back to merge proposal