Hey Jeff,
On 01.07.2008, at 22:11, Jeff Hammel wrote:
> Also, if I wasn't verbose enough about it in the first email, I'd
> love feedback on how to do anything I'm doing here better so that I
> can write better code in the future.
When asking for patch review on the mailing list, including the patch
inline would be helpful ;)
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/
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
