Jan Schukat wrote: > Ok, so best would probably be, to include add the decorator > definitions to the trunk, since they don't interfere with anything, > maybe without the WikiPage adjustments in delete and save. And then > the new db functionality, like my rename stuff, can start using it, so > we can get experience. > > Attached is a diff, that just contains the decorators. >
This time the patch was fine for Thunderbird ;-) Some additional comments I could have made for the previous patch as well: - from the rest of discussion, you should have seen that Remy and me agreed on a version without the "raise_exceptions" approach, so you shouldn't have reintroduced that, at least not without argumentation; same thing for the "return" part, as I already commented - you introduced two very similar decorators, with_transaction and with_db_transaction; the only benefit of the with_transaction one is to get a cursor from the db, which is something very trivial to do anyway (db.cursor()), so I don't think this warrants the extra complexity of having *2* decorators; with_db_transaction is enough and should be renamed with_transaction. > So either apply this patch, or the one in the mail before with the > changed WikiPage. Either way, some conclusion would be nice, so it's > clear how to proceed. > > Once we find the patch satisfying (i.e. all the feedback taken into account), we'll integrate it in trunk or perhaps first in a tmp branch, as we see fit. (Remy wrote) >> I wouldn't convert all database writes to the new scheme in one big >> patch. Agreed. >> Instead, I would rather introduce it with new code (IIRC, you >> wanted to work on wiki page renames) and get familiar with it, maybe >> refine the scheme somewhat with the experience gained, and only then, >> transition old code progressively. >> >> Or, as there were also a bunch of #1106 specific issues to address first, do it for one module, exactly like you did in your transactions_py2.4.diff patch. >> Converting all writes at the same time sounds like a good way to >> introduce subtle bugs all over the code. Instead, if the new >> functionality is initially confined to a small section of code, the >> potential damage should be confined as well. >> >> Also agreed. But we might want to make all these changes on a branch, in order to get an overview of all the changes implied. In particular, I'm wondering how this will interact with #6348 (http://trac.edgewall.org/ticket/6348), which I should really get around to ... -- Christian --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
