On 01.07.2008, at 22:11, Jeff Hammel wrote:

On the patch itself:

* What's the use case for configurable default query_args? Couldn't  
the default options for the query module itself be reused instead, if  
possible? Or just some sane default like all open tickets?

* The excluded_fields option refers to fields not included with Trac  
core. This should probably default to an empty list instead, if it is  
even necessary at all (note how I like to avoid adding config  
options :P )

* The option group should probably be just "ticket" instead of  
"autoquery"

* Don't use self.env.href, use req.href.

* The query_link thing is kind of brute force. Would be nicer to just  
use the **kwargs of req.href to construct the URL qith query string  
params. And it probably should be `query_link(owner=ticket.owner)`  
instead of `query_link('owner', ticket.owner)`. It would probably be  
possible to have query_link be just a partial binding of req.href.

* The template has two py:if="" directives that serve as if/else.  
Should be using py:choose instead.

On a related note, I'd prefer if the links in the ticket properties  
box (the yellow box) would be more subtle (I'm assuming they're the  
default red right now), possibly even only really discoverable on  
hover. Many pages in Trac have already gotten way to "linky"; at some  
point you end up with content where every second word is a red link,  
which is ugly and distracting :P

Cheers,
--
Christopher Lenz
   cmlenz at gmx.de
   http://www.cmlenz.net/