Hi Mikael, > I'm just bumping this thread since there has not been any response to the > latest patch I supplied about three weeks ago > (http://trac.edgewall.org/ticket/6492#comment:37).
Yes, a reply was overdue, sorry about that. > I would like to see this feature in Trac 0.13, so what do you guys think is > necessary for me to do to make the patches accepted? First, I like the visuals very much. The icons are nice and clean. I'm not completely sure about the bold attribute for unseen events, but that's a matter of taste, and I'm fine with it. About the code, I have the following comments, of which only the first one is important: - I like the idea of making the timeline even a class. However, I don't think the rendering code should be moved to those classes. Timeline events can be consumed by other components, possibly unrelated to HTML rendering. So I would prefer the events to stay "pure data" objects, and the rendering code to remain in the respective rendering components. That would also cut down on the number of new classes, as most providers would just yield TimelineEvent instances. - The ticket_inform_unfixed and ticket_warn_unfixed options could be made ListOption instances, to make use of the automatic conversion to a list. - To update an object's __dict__ from another dict, you can use (in TimelineEvent): self.__dict__.update(kw) - any() can be passed an iterator, so instead of: any([a in req.args for a in ...]) you can write: any(a in req.args for a in ...) Let's hear what others have to say! -- Remy
signature.asc
Description: OpenPGP digital signature
