Agreed with all nitpicks, and will prepare new patch on Sunday (I'm away now).
Except, what is the issue with the new_text variable? That's my way of moving validity checks out of the transaction, since they don't belong there and should be done before the transaction is started. I could make the checks multiple times though, first to rule out the error case, and then to distinguish between the two update types. Just makes the code longer and less clear IMHO. Regards Jan Remy Blank <[email protected]> schrieb am 16.10.2009 22:28:19: > Christian Boos wrote: > > Hey, this one looks great! > > I have got some minor nitpicks: > > - Use WikiFormatting for docstrings. > > - The "try: except: raise" in the case where db is not None is not > necessary, just call fn(db). > > - Sort imports alphabetically. > > - Use "cursor" for cursors. That's the convention for the rest of Trac. > > - There's still an "if(new_text)" in save(). > > > Remy, OK for an experimental branch, starting with this change? > > A branch sounds a bit overkill for this, I would have opened a ticket > and tracked progress with patches, and integrated the result into trunk. > But I'm ok if you think it is worth the overhead. > > -- Remy > > > --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
